From a2374766facdbcfe55d37f3a795da35df590a6ce Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Mon, 26 Jun 2023 17:03:39 -0400 Subject: [PATCH 01/29] Add truncate_pr_description and align rescue properly in fetch_file_contents method --- common/lib/dependabot/clients/codecommit.rb | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/common/lib/dependabot/clients/codecommit.rb b/common/lib/dependabot/clients/codecommit.rb index 18ec4c32df4..297f04d0bd8 100644 --- a/common/lib/dependabot/clients/codecommit.rb +++ b/common/lib/dependabot/clients/codecommit.rb @@ -7,6 +7,8 @@ module Clients class CodeCommit class NotFound < StandardError; end + MAX_PR_DESCRIPTION_LENGTH = 10_239 + ####################### # Constructor methods # ####################### @@ -68,8 +70,8 @@ def fetch_file_contents(repo, commit, path) commit_specifier: commit, file_path: path ).file_content - rescue Aws::CodeCommit::Errors::FileDoesNotExistException - raise NotFound + rescue Aws::CodeCommit::Errors::FileDoesNotExistException + raise NotFound end def branch(branch_name) @@ -182,6 +184,7 @@ def create_commit(branch_name, author_name, base_commit, commit_message, def create_pull_request(pr_name, target_branch, source_branch, pr_description) + pr_description = truncate_pr_description(pr_description) cc_client.create_pull_request( title: pr_name, description: pr_description, @@ -193,6 +196,19 @@ def create_pull_request(pr_name, target_branch, source_branch, ) end + def truncate_pr_description(pr_description) + # Codecommit only supports descriptions up to 10240 chars in length + # https://docs.aws.amazon.com/codecommit/latest/APIReference/API_PullRequest.html + pr_description = pr_description.to_s + if pr_description.length > MAX_PR_DESCRIPTION_LENGTH + truncated_msg = "...\n\n_Description has been truncated_" + truncate_length = MAX_PR_DESCRIPTION_LENGTH - truncated_msg.length + pr_description = (pr_description[0..truncate_length] + truncated_msg) + end + + pr_description + end + private attr_reader :credentials From 06461cdbaad5d2be4f2848c8f1eba1581d2fb32a Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Wed, 28 Jun 2023 21:56:48 -0400 Subject: [PATCH 02/29] Add specs for truncate method --- .../dependabot/clients/codecommit_spec.rb | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/common/spec/dependabot/clients/codecommit_spec.rb b/common/spec/dependabot/clients/codecommit_spec.rb index ddbfefcb892..11c267ec301 100644 --- a/common/spec/dependabot/clients/codecommit_spec.rb +++ b/common/spec/dependabot/clients/codecommit_spec.rb @@ -76,4 +76,49 @@ end end end + + describe "#truncate_pr_description" do + subject { described_class.new.truncate_pr_description(pr_description) } + + let(:pr_description) { "This is a normal length PR description and it should not be truncated." } + + context "when the pull request description is within the allowed length" do + it "returns the original description" do + expect(subject).to eq(pr_description) + end + end + + context "when the pull request description exceeds the allowed length" do + let(:pr_description) { "A" * 11_000 } # Exceeds the maximum length of 10,239 + + it "truncates the description and appends a truncated message" do + expected_truncated_description = "#{pr_description[0..10_238]}...\n\n_Description has been truncated_" + expect(subject).to eq(expected_truncated_description) + end + end + + context "when the pull request description is already truncated" do + let(:pr_description) { "This is a truncated description...\n\n_Description has been truncated_" } + + it "returns the original truncated description" do + expect(subject).to eq(pr_description) + end + end + + context "when the pull request description is an empty string" do + let(:pr_description) { "" } + + it "returns an empty string" do + expect(subject).to eq("") + end + end + + context "when the pull request description is nil" do + let(:pr_description) { nil } + + it "returns an empty string" do + expect(subject).to eq("") + end + end + end end From 55c1ac3779b7bed39e06ab309b59ee6bab5908b1 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Wed, 28 Jun 2023 22:04:03 -0400 Subject: [PATCH 03/29] use stubbed client --- common/spec/dependabot/clients/codecommit_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/spec/dependabot/clients/codecommit_spec.rb b/common/spec/dependabot/clients/codecommit_spec.rb index 11c267ec301..4e4d7a22179 100644 --- a/common/spec/dependabot/clients/codecommit_spec.rb +++ b/common/spec/dependabot/clients/codecommit_spec.rb @@ -78,7 +78,7 @@ end describe "#truncate_pr_description" do - subject { described_class.new.truncate_pr_description(pr_description) } + subject { stubbed_cc_client.truncate_pr_description(pr_description) } let(:pr_description) { "This is a normal length PR description and it should not be truncated." } From 1540d6d51f38d352b804c57d2e844e00f8ceb879 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Wed, 28 Jun 2023 22:12:43 -0400 Subject: [PATCH 04/29] Use client --- common/spec/dependabot/clients/codecommit_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/spec/dependabot/clients/codecommit_spec.rb b/common/spec/dependabot/clients/codecommit_spec.rb index 4e4d7a22179..8089288e146 100644 --- a/common/spec/dependabot/clients/codecommit_spec.rb +++ b/common/spec/dependabot/clients/codecommit_spec.rb @@ -78,7 +78,7 @@ end describe "#truncate_pr_description" do - subject { stubbed_cc_client.truncate_pr_description(pr_description) } + subject { client.truncate_pr_description(pr_description) } let(:pr_description) { "This is a normal length PR description and it should not be truncated." } From 25b499e47710097071e7e65a7efd1afc0bb8d6b3 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Wed, 28 Jun 2023 22:21:15 -0400 Subject: [PATCH 05/29] Fix logic on comparing truncated strings --- common/spec/dependabot/clients/codecommit_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/common/spec/dependabot/clients/codecommit_spec.rb b/common/spec/dependabot/clients/codecommit_spec.rb index 8089288e146..cf41686068a 100644 --- a/common/spec/dependabot/clients/codecommit_spec.rb +++ b/common/spec/dependabot/clients/codecommit_spec.rb @@ -92,7 +92,9 @@ let(:pr_description) { "A" * 11_000 } # Exceeds the maximum length of 10,239 it "truncates the description and appends a truncated message" do - expected_truncated_description = "#{pr_description[0..10_238]}...\n\n_Description has been truncated_" + truncated_msg = "...\n\n_Description has been truncated_" + truncate_length = 10_239 - truncated_msg.length + expected_truncated_description = "#{pr_description[0..truncate_length]}#{truncated_msg}" expect(subject).to eq(expected_truncated_description) end end From 09ad9120479b8d332af87aedee20e01217f4d6f3 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 14:11:58 -0400 Subject: [PATCH 06/29] Move truncate to message builder and pass in the default values if no values have been set for max length or encoding --- common/lib/dependabot/clients/azure.rb | 14 ---- common/lib/dependabot/clients/codecommit.rb | 16 ---- common/lib/dependabot/pull_request_creator.rb | 21 +++++- .../dependabot/pull_request_creator/azure.rb | 5 ++ .../pull_request_creator/codecommit.rb | 3 + .../dependabot/pull_request_creator/github.rb | 18 ++--- .../pull_request_creator/message_builder.rb | 31 +++++++- .../dependabot/clients/codecommit_spec.rb | 47 ------------ .../message_builder_spec.rb | 75 +++++++++++++++++++ 9 files changed, 133 insertions(+), 97 deletions(-) diff --git a/common/lib/dependabot/clients/azure.rb b/common/lib/dependabot/clients/azure.rb index c43df865b19..1da8540fc39 100644 --- a/common/lib/dependabot/clients/azure.rb +++ b/common/lib/dependabot/clients/azure.rb @@ -174,7 +174,6 @@ def create_commit(branch_name, base_commit, commit_message, files, def create_pull_request(pr_name, source_branch, target_branch, pr_description, labels, reviewers = nil, assignees = nil, work_item = nil) - pr_description = truncate_pr_description(pr_description) content = { sourceRefName: "refs/heads/" + source_branch, @@ -375,19 +374,6 @@ def auth_header_for(token) end end - def truncate_pr_description(pr_description) - # Azure DevOps only support descriptions up to 4000 characters in UTF-16 - # encoding. - # https://developercommunity.visualstudio.com/content/problem/608770/remove-4000-character-limit-on-pull-request-descri.html - pr_description = pr_description.dup.force_encoding(Encoding::UTF_16) - if pr_description.length > MAX_PR_DESCRIPTION_LENGTH - truncated_msg = (+"...\n\n_Description has been truncated_").force_encoding(Encoding::UTF_16) - truncate_length = MAX_PR_DESCRIPTION_LENGTH - truncated_msg.length - pr_description = (pr_description[0..truncate_length] + truncated_msg) - end - pr_description.force_encoding(Encoding::UTF_8) - end - def tags_creation_forbidden?(response) return if response.body.empty? diff --git a/common/lib/dependabot/clients/codecommit.rb b/common/lib/dependabot/clients/codecommit.rb index 297f04d0bd8..fd5f92ecfca 100644 --- a/common/lib/dependabot/clients/codecommit.rb +++ b/common/lib/dependabot/clients/codecommit.rb @@ -7,8 +7,6 @@ module Clients class CodeCommit class NotFound < StandardError; end - MAX_PR_DESCRIPTION_LENGTH = 10_239 - ####################### # Constructor methods # ####################### @@ -184,7 +182,6 @@ def create_commit(branch_name, author_name, base_commit, commit_message, def create_pull_request(pr_name, target_branch, source_branch, pr_description) - pr_description = truncate_pr_description(pr_description) cc_client.create_pull_request( title: pr_name, description: pr_description, @@ -196,19 +193,6 @@ def create_pull_request(pr_name, target_branch, source_branch, ) end - def truncate_pr_description(pr_description) - # Codecommit only supports descriptions up to 10240 chars in length - # https://docs.aws.amazon.com/codecommit/latest/APIReference/API_PullRequest.html - pr_description = pr_description.to_s - if pr_description.length > MAX_PR_DESCRIPTION_LENGTH - truncated_msg = "...\n\n_Description has been truncated_" - truncate_length = MAX_PR_DESCRIPTION_LENGTH - truncated_msg.length - pr_description = (pr_description[0..truncate_length] + truncated_msg) - end - - pr_description - end - private attr_reader :credentials diff --git a/common/lib/dependabot/pull_request_creator.rb b/common/lib/dependabot/pull_request_creator.rb index f5e49a5da60..37c5c9450c1 100644 --- a/common/lib/dependabot/pull_request_creator.rb +++ b/common/lib/dependabot/pull_request_creator.rb @@ -49,7 +49,7 @@ def initialize(cause, pull_request) :commit_message_options, :vulnerabilities_fixed, :reviewers, :assignees, :milestone, :branch_name_separator, :branch_name_prefix, :branch_name_max_length, :github_redirection_service, - :custom_headers, :provider_metadata, :dependency_group + :custom_headers, :provider_metadata, :dependency_group, :pr_message_max_length def initialize(source:, base_commit:, dependencies:, files:, credentials:, pr_message_header: nil, pr_message_footer: nil, @@ -61,7 +61,7 @@ def initialize(source:, base_commit:, dependencies:, files:, credentials:, automerge_candidate: false, github_redirection_service: DEFAULT_GITHUB_REDIRECTION_SERVICE, custom_headers: nil, require_up_to_date_base: false, - provider_metadata: {}, message: nil, dependency_group: nil) + provider_metadata: {}, message: nil, dependency_group: nil, pr_message_max_length: nil) @dependencies = dependencies @source = source @base_commit = base_commit @@ -88,6 +88,7 @@ def initialize(source:, base_commit:, dependencies:, files:, credentials:, @provider_metadata = provider_metadata @message = message @dependency_group = dependency_group + @pr_message_max_length = pr_message_max_length check_dependencies_have_previous_version end @@ -216,6 +217,18 @@ def codecommit_creator end def message + @message unless @message.nil? + + case source.provider + when "github" + pr_message_max_length = Github::MAX_PR_DESCRIPTION_LENGTH if pr_message_max_length.nil? + when "azure" + pr_message_max_length = Azure::MAX_PR_DESCRIPTION_LENGTH if pr_message_max_length.nil? + pr_message_encoding = Azure::ENCODING if pr_message_encoding.nil? + when "codecommit" + pr_message_max_length = Codecommit::MAX_PR_DESCRIPTION_LENGTH if pr_message_max_length.nil? + end + @message ||= MessageBuilder.new( source: source, @@ -227,7 +240,9 @@ def message pr_message_footer: pr_message_footer, vulnerabilities_fixed: vulnerabilities_fixed, github_redirection_service: github_redirection_service, - dependency_group: dependency_group + dependency_group: dependency_group, + pr_message_max_length: pr_message_max_length, + pr_message_encoding: pr_message_encoding ) end diff --git a/common/lib/dependabot/pull_request_creator/azure.rb b/common/lib/dependabot/pull_request_creator/azure.rb index 1a139bf3631..4c1e9a14524 100644 --- a/common/lib/dependabot/pull_request_creator/azure.rb +++ b/common/lib/dependabot/pull_request_creator/azure.rb @@ -10,6 +10,11 @@ class Azure :files, :commit_message, :pr_description, :pr_name, :author_details, :labeler, :reviewers, :assignees, :work_item + MAX_PR_DESCRIPTION_LENGTH = 3999 # Azure has a max length of 4000 (0 based count) + ENCODING = Encoding::UTF_16 # Azure DevOps only support descriptions up to 4000 characters in UTF-16 + # encoding. + # https://developercommunity.visualstudio.com/content/problem/608770/remove-4000-character-limit-on-pull-request-descri.html + def initialize(source:, branch_name:, base_commit:, credentials:, files:, commit_message:, pr_description:, pr_name:, author_details:, labeler:, reviewers: nil, assignees: nil, work_item: nil) diff --git a/common/lib/dependabot/pull_request_creator/codecommit.rb b/common/lib/dependabot/pull_request_creator/codecommit.rb index e7bb305ea33..d542d61cdea 100644 --- a/common/lib/dependabot/pull_request_creator/codecommit.rb +++ b/common/lib/dependabot/pull_request_creator/codecommit.rb @@ -10,6 +10,9 @@ class Codecommit :files, :commit_message, :pr_description, :pr_name, :author_details, :labeler + MAX_PR_DESCRIPTION_LENGTH = 10_239 # CodeCommit has a max length of 10240 (0 based count) + # https://docs.aws.amazon.com/codecommit/latest/APIReference/API_PullRequest.html + def initialize(source:, branch_name:, base_commit:, credentials:, files:, commit_message:, pr_description:, pr_name:, author_details:, labeler:, require_up_to_date_base:) diff --git a/common/lib/dependabot/pull_request_creator/github.rb b/common/lib/dependabot/pull_request_creator/github.rb index 0ea6e2546e6..c58d85b5801 100644 --- a/common/lib/dependabot/pull_request_creator/github.rb +++ b/common/lib/dependabot/pull_request_creator/github.rb @@ -9,7 +9,11 @@ module Dependabot class PullRequestCreator # rubocop:disable Metrics/ClassLength class Github - MAX_PR_DESCRIPTION_LENGTH = 65_536 # characters (see #create_pull_request) + MAX_PR_DESCRIPTION_LENGTH = 65_536 # Limit PR description to MAX_PR_DESCRIPTION_LENGTH (65,536) characters + # and truncate with message if over. The API limit is 262,144 bytes + # (https://github.community/t/maximum-length-for-the-comment-body-in-issues-and-pr/148867/2). + # As Ruby strings are UTF-8 encoded, this is a pessimistic limit: it + # presumes the case where all characters are 4 bytes. attr_reader :source, :branch_name, :base_commit, :credentials, :files, :pr_description, :pr_name, :commit_message, @@ -349,18 +353,6 @@ def add_milestone_to_pull_request(pull_request) end def create_pull_request - # Limit PR description to MAX_PR_DESCRIPTION_LENGTH (65,536) characters - # and truncate with message if over. The API limit is 262,144 bytes - # (https://github.community/t/maximum-length-for-the-comment-body-in-issues-and-pr/148867/2). - # As Ruby strings are UTF-8 encoded, this is a pessimistic limit: it - # presumes the case where all characters are 4 bytes. - pr_description = @pr_description.dup - if pr_description && pr_description.length > MAX_PR_DESCRIPTION_LENGTH - truncated_msg = "...\n\n_Description has been truncated_" - truncate_length = MAX_PR_DESCRIPTION_LENGTH - truncated_msg.length - pr_description = (pr_description[0, truncate_length] + truncated_msg) - end - github_client_for_source.create_pull_request( source.repo, target_branch, diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index edd04d0521a..e7d07758fd1 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -22,13 +22,16 @@ class MessageBuilder attr_reader :source, :dependencies, :files, :credentials, :pr_message_header, :pr_message_footer, :commit_message_options, :vulnerabilities_fixed, - :github_redirection_service, :dependency_group + :github_redirection_service, :dependency_group, :pr_message_max_length, + :pr_message_encoding + + TRUNCATED_MSG = "...\n\n_Description has been truncated_" def initialize(source:, dependencies:, files:, credentials:, pr_message_header: nil, pr_message_footer: nil, commit_message_options: {}, vulnerabilities_fixed: {}, github_redirection_service: DEFAULT_GITHUB_REDIRECTION_SERVICE, - dependency_group: nil) + dependency_group: nil, pr_message_max_length: nil, pr_message_encoding: nil) @dependencies = dependencies @files = files @source = source @@ -39,6 +42,8 @@ def initialize(source:, dependencies:, files:, credentials:, @vulnerabilities_fixed = vulnerabilities_fixed @github_redirection_service = github_redirection_service @dependency_group = dependency_group + @pr_message_max_length = pr_message_max_length + @pr_message_encoding = pr_message_encoding end def pr_name @@ -48,13 +53,31 @@ def pr_name end def pr_message - suffixed_pr_message_header + commit_message_intro + - metadata_cascades + prefixed_pr_message_footer + msg = "#{suffixed_pr_message_header}#{commit_message_intro}#{metadata_cascades}#{prefixed_pr_message_footer}" + truncate_pr_message(msg) rescue StandardError => e Dependabot.logger.error("Error while generating PR message: #{e.message}") suffixed_pr_message_header + prefixed_pr_message_footer end + # This method accepts a message, a max length and an optional encoding + # If the max length is not positive or the message length is less than max_length + # the original message is returned, otherwise it will optionally encode the message + # truncate it, then optional reencode back to the ruby standard of UTF_8 + def truncate_pr_message(msg) + return msg if pr_message_max_length.nil? + + msg = msg.dup + msg = msg.force_encoding(pr_message_encoding) unless pr_message_encoding.nil? + + if msg.length > pr_message_max_length + trunc_msg = pr_message_encoding.nil? ? TRUNCATED_MSG : (+TRUNCATED_MSG).dup.force_encoding(pr_message_encoding) + trunc_length = pr_message_max_length - trunc_msg.length + msg = (msg[0..trunc_length] + trunc_msg) + end + msg.force_encoding(Encoding::UTF_8) unless pr_message_encoding.nil? + end + def commit_message message = commit_subject + "\n\n" message += commit_message_intro diff --git a/common/spec/dependabot/clients/codecommit_spec.rb b/common/spec/dependabot/clients/codecommit_spec.rb index cf41686068a..ddbfefcb892 100644 --- a/common/spec/dependabot/clients/codecommit_spec.rb +++ b/common/spec/dependabot/clients/codecommit_spec.rb @@ -76,51 +76,4 @@ end end end - - describe "#truncate_pr_description" do - subject { client.truncate_pr_description(pr_description) } - - let(:pr_description) { "This is a normal length PR description and it should not be truncated." } - - context "when the pull request description is within the allowed length" do - it "returns the original description" do - expect(subject).to eq(pr_description) - end - end - - context "when the pull request description exceeds the allowed length" do - let(:pr_description) { "A" * 11_000 } # Exceeds the maximum length of 10,239 - - it "truncates the description and appends a truncated message" do - truncated_msg = "...\n\n_Description has been truncated_" - truncate_length = 10_239 - truncated_msg.length - expected_truncated_description = "#{pr_description[0..truncate_length]}#{truncated_msg}" - expect(subject).to eq(expected_truncated_description) - end - end - - context "when the pull request description is already truncated" do - let(:pr_description) { "This is a truncated description...\n\n_Description has been truncated_" } - - it "returns the original truncated description" do - expect(subject).to eq(pr_description) - end - end - - context "when the pull request description is an empty string" do - let(:pr_description) { "" } - - it "returns an empty string" do - expect(subject).to eq("") - end - end - - context "when the pull request description is nil" do - let(:pr_description) { nil } - - it "returns an empty string" do - expect(subject).to eq("") - end - end - end end diff --git a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb index e9197229784..20de6b35efc 100644 --- a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb +++ b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb @@ -2572,4 +2572,79 @@ def commits_details(base:, head:) its(:pr_message) { should eq(pr_message) } its(:commit_message) { should eq(commit_message) } end + + describe "#truncate_pr_description" do + subject { builder.truncate_pr_description(pr_description) } + + let(:pr_description) { "This is a normal length PR description and it should not be truncated." } + + context "when the pull request description is nil" do + let(:pr_description) { nil } + + it "returns an empty string" do + expect(subject).to eq("") + end + end + end + + subject(:message_builder) { described_class.new(source: source, dependencies: dependencies) } + + let(:source) { instance_double("Dependabot::Source") } + let(:dependencies) { [instance_double("Dependabot::Dependency")] } + let(:message) { "This is a normal length PR description and it should not be truncated." } + + describe "#truncate_pr_message" do + context "when pr_message_max_length is not provided" do + it "returns the original message" do + expect(message_builder.truncate_pr_message(message)).to eq(message) + end + + let(:message) { "This is a test message with special characters: © ®" } + it "returns the original encoding of the message" do + message_builder.pr_message_encoding = Encoding::UTF_16 + expect(message_builder.truncate_pr_message(message)).to eq(message) + end + end + + context "when pr_message_max_length is provided" do + let(:message) { "A" * 11_000 } # Exceeds the maximum length of 10,239 + let(:pr_message_max_length) { 10_239 } + it "truncates the message to the specified length" do + truncated_msg = "...\n\n_Description has been truncated_" + truncate_length = 10_239 - truncated_msg.length + expected_truncated_description = "#{pr_description[0..truncate_length]}#{truncated_msg}" + + message_builder.pr_message_max_length = pr_message_max_length + expect(message_builder.truncate_pr_message(message)).to eq(expected_truncated_description) + end + + let(:message) { "© ®" * 100 } # Exceeds the maximum length of 100 + let(:pr_message_max_length) { 100 } + it "truncates and maintains the specified encoding" do + msg = message.dup.force_encoding(Encoding::UTF_16) + trunc_msg = (+"...\n\n_Description has been truncated_").force_encoding(Encoding::UTF_16) + trunc_length = pr_message_max_length - trunc_msg.length + msg = (msg[0..trunc_length] + trunc_msg) + msg = msg.force_encoding(Encoding::UTF_8) + + message_builder.pr_message_max_length = pr_message_max_length + message_builder.pr_message_encoding = Encoding::UTF_16 + expect(message_builder.truncate_pr_message(message)).to eq(msg) + end + end + + context "when the pull request description is an empty string" do + let(:message) { "" } + let(:pr_message_max_length) { 100 } + it "returns an empty string" do + message_builder.pr_message_max_length = pr_message_max_length + expect(message_builder.truncate_pr_message(message)).to eq("") + end + it "returns an empty string when encoded" do + message_builder.pr_message_max_length = pr_message_max_length + message_builder.pr_message_encoding = Encoding::UTF_16 + expect(message_builder.truncate_pr_message(message)).to eq("") + end + end + end end From ac9ddc5f84165f6eb6fb1ea3be824f39ea0942e3 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 14:21:31 -0400 Subject: [PATCH 07/29] Remove spec that was unintentionally added --- .../pull_request_creator/message_builder_spec.rb | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb index 20de6b35efc..99b19772e15 100644 --- a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb +++ b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb @@ -2573,20 +2573,6 @@ def commits_details(base:, head:) its(:commit_message) { should eq(commit_message) } end - describe "#truncate_pr_description" do - subject { builder.truncate_pr_description(pr_description) } - - let(:pr_description) { "This is a normal length PR description and it should not be truncated." } - - context "when the pull request description is nil" do - let(:pr_description) { nil } - - it "returns an empty string" do - expect(subject).to eq("") - end - end - end - subject(:message_builder) { described_class.new(source: source, dependencies: dependencies) } let(:source) { instance_double("Dependabot::Source") } From 62a8f76fb10c91adc183ee51c50c00c5536c9db4 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 14:29:02 -0400 Subject: [PATCH 08/29] Remove the old pr description tests --- .../pull_request_creator/azure_spec.rb | 17 ----------------- .../pull_request_creator/github_spec.rb | 19 ------------------- .../message_builder_spec.rb | 2 +- 3 files changed, 1 insertion(+), 37 deletions(-) diff --git a/common/spec/dependabot/pull_request_creator/azure_spec.rb b/common/spec/dependabot/pull_request_creator/azure_spec.rb index 1996ad7a38d..d6d9fed1c21 100644 --- a/common/spec/dependabot/pull_request_creator/azure_spec.rb +++ b/common/spec/dependabot/pull_request_creator/azure_spec.rb @@ -168,23 +168,6 @@ end end - context "with e very long pr description" do - let(:pr_description) { ("a" * 3997) + "💣 kaboom" } - it "truncates the description respecting azures encoding" do - creator.create - - expect(WebMock). - to( - have_requested(:post, "#{repo_api_url}/pullrequests?api-version=5.0"). - with do |req| - description = JSON.parse(req.body).fetch("description") - expect(description.length).to eq 4000 - expect(description).to end_with("\n\n_Description has been truncated_") - end - ) - end - end - context "with author details provided" do let(:author_details) do { email: "support@dependabot.com", name: "dependabot" } diff --git a/common/spec/dependabot/pull_request_creator/github_spec.rb b/common/spec/dependabot/pull_request_creator/github_spec.rb index ba472aa6b09..93bb17bad26 100644 --- a/common/spec/dependabot/pull_request_creator/github_spec.rb +++ b/common/spec/dependabot/pull_request_creator/github_spec.rb @@ -1088,25 +1088,6 @@ ) end end - - context "the PR description is too long" do - let(:pr_description) { "a" * (described_class::MAX_PR_DESCRIPTION_LENGTH + 1) } - - it "truncates the description" do - creator.create - - expect(WebMock). - to have_requested(:post, "#{repo_api_url}/pulls"). - with( - body: { - base: "master", - head: "dependabot/bundler/business-1.5.0", - title: "PR name", - body: ->(body) { expect(body.length).to be <= described_class::MAX_PR_DESCRIPTION_LENGTH } - } - ) - end - end end end end diff --git a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb index 99b19772e15..268ca63fa70 100644 --- a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb +++ b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb @@ -2593,7 +2593,7 @@ def commits_details(base:, head:) end context "when pr_message_max_length is provided" do - let(:message) { "A" * 11_000 } # Exceeds the maximum length of 10,239 + let(:message) { "A" * 10_250 } # Exceeds the maximum length of 10,239 let(:pr_message_max_length) { 10_239 } it "truncates the message to the specified length" do truncated_msg = "...\n\n_Description has been truncated_" From 946b1eb2a6f16d37386be110a65bb20415c36ce6 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 14:34:49 -0400 Subject: [PATCH 09/29] Reuse properly configured builder --- .../pull_request_creator/message_builder_spec.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb index 268ca63fa70..5b65c1e2158 100644 --- a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb +++ b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb @@ -2573,14 +2573,10 @@ def commits_details(base:, head:) its(:commit_message) { should eq(commit_message) } end - subject(:message_builder) { described_class.new(source: source, dependencies: dependencies) } - - let(:source) { instance_double("Dependabot::Source") } - let(:dependencies) { [instance_double("Dependabot::Dependency")] } - let(:message) { "This is a normal length PR description and it should not be truncated." } - + subject(:message_builder) { builder } describe "#truncate_pr_message" do context "when pr_message_max_length is not provided" do + let(:message) { "This is a normal length PR description and it should not be truncated." } it "returns the original message" do expect(message_builder.truncate_pr_message(message)).to eq(message) end From 6c126c7e11a5d47afdfb2276ad5b6fcdd9a7b019 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 14:40:14 -0400 Subject: [PATCH 10/29] Fix reference to undefined variable --- .../dependabot/pull_request_creator/message_builder_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb index 5b65c1e2158..368105b646d 100644 --- a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb +++ b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb @@ -2594,7 +2594,7 @@ def commits_details(base:, head:) it "truncates the message to the specified length" do truncated_msg = "...\n\n_Description has been truncated_" truncate_length = 10_239 - truncated_msg.length - expected_truncated_description = "#{pr_description[0..truncate_length]}#{truncated_msg}" + expected_truncated_description = "#{message[0..truncate_length]}#{truncated_msg}" message_builder.pr_message_max_length = pr_message_max_length expect(message_builder.truncate_pr_message(message)).to eq(expected_truncated_description) From 7429f9eccafb5c50af99987b17708c2e60b15de3 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 15:01:13 -0400 Subject: [PATCH 11/29] Add setter for new fields --- common/lib/dependabot/pull_request_creator/message_builder.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index e7d07758fd1..9e50b1448f6 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -46,6 +46,10 @@ def initialize(source:, dependencies:, files:, credentials:, @pr_message_encoding = pr_message_encoding end + def self.pr_message_max_length=(max_length) + + def self.pr_message_encoding=(max_length) + def pr_name name = dependency_group ? group_pr_name : solo_pr_name name[0] = name[0].capitalize if pr_name_prefixer.capitalize_first_word? From 514f5e21980a8dc21ff9ab0b4fd47b12cb2be339 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 15:09:04 -0400 Subject: [PATCH 12/29] Fix setters --- common/lib/dependabot/pull_request_creator/message_builder.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index 9e50b1448f6..ff8c0df2ca0 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -46,9 +46,9 @@ def initialize(source:, dependencies:, files:, credentials:, @pr_message_encoding = pr_message_encoding end - def self.pr_message_max_length=(max_length) + attr_writer :pr_message_max_length - def self.pr_message_encoding=(max_length) + attr_writer :pr_message_encoding def pr_name name = dependency_group ? group_pr_name : solo_pr_name From 13d79f0cec28150d5875b11cc987899224be1a51 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 15:13:55 -0400 Subject: [PATCH 13/29] return msg --- common/lib/dependabot/pull_request_creator/message_builder.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index ff8c0df2ca0..7155ea982fa 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -80,6 +80,7 @@ def truncate_pr_message(msg) msg = (msg[0..trunc_length] + trunc_msg) end msg.force_encoding(Encoding::UTF_8) unless pr_message_encoding.nil? + msg end def commit_message From 610cae07e6c72524bdc13c977a298ad88a4ee5c4 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 15:38:06 -0400 Subject: [PATCH 14/29] Add consistency to how max length is used, create encode variable --- .../message_builder_spec.rb | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb index 368105b646d..ea22129b3be 100644 --- a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb +++ b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb @@ -2577,11 +2577,13 @@ def commits_details(base:, head:) describe "#truncate_pr_message" do context "when pr_message_max_length is not provided" do let(:message) { "This is a normal length PR description and it should not be truncated." } + it "returns the original message" do expect(message_builder.truncate_pr_message(message)).to eq(message) end let(:message) { "This is a test message with special characters: © ®" } + it "returns the original encoding of the message" do message_builder.pr_message_encoding = Encoding::UTF_16 expect(message_builder.truncate_pr_message(message)).to eq(message) @@ -2591,9 +2593,10 @@ def commits_details(base:, head:) context "when pr_message_max_length is provided" do let(:message) { "A" * 10_250 } # Exceeds the maximum length of 10,239 let(:pr_message_max_length) { 10_239 } + it "truncates the message to the specified length" do truncated_msg = "...\n\n_Description has been truncated_" - truncate_length = 10_239 - truncated_msg.length + truncate_length = pr_message_max_length - truncated_msg.length expected_truncated_description = "#{message[0..truncate_length]}#{truncated_msg}" message_builder.pr_message_max_length = pr_message_max_length @@ -2602,15 +2605,17 @@ def commits_details(base:, head:) let(:message) { "© ®" * 100 } # Exceeds the maximum length of 100 let(:pr_message_max_length) { 100 } + it "truncates and maintains the specified encoding" do - msg = message.dup.force_encoding(Encoding::UTF_16) - trunc_msg = (+"...\n\n_Description has been truncated_").force_encoding(Encoding::UTF_16) + encode_utf16 = Encoding::UTF_16 + msg = message.dup.force_encoding + trunc_msg = (+"...\n\n_Description has been truncated_").force_encoding(encode_utf16) trunc_length = pr_message_max_length - trunc_msg.length - msg = (msg[0..trunc_length] + trunc_msg) + msg = "#{msg[0..trunc_length]}#{trunc_msg}" msg = msg.force_encoding(Encoding::UTF_8) message_builder.pr_message_max_length = pr_message_max_length - message_builder.pr_message_encoding = Encoding::UTF_16 + message_builder.pr_message_encoding = encode_utf16 expect(message_builder.truncate_pr_message(message)).to eq(msg) end end @@ -2618,10 +2623,12 @@ def commits_details(base:, head:) context "when the pull request description is an empty string" do let(:message) { "" } let(:pr_message_max_length) { 100 } + it "returns an empty string" do message_builder.pr_message_max_length = pr_message_max_length expect(message_builder.truncate_pr_message(message)).to eq("") end + it "returns an empty string when encoded" do message_builder.pr_message_max_length = pr_message_max_length message_builder.pr_message_encoding = Encoding::UTF_16 From 0c087c17e26a395f17be048ae3aa5b107dd8769c Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 16:02:53 -0400 Subject: [PATCH 15/29] Fix lint and add param to encode --- .../lib/dependabot/pull_request_creator/message_builder.rb | 6 +++--- .../dependabot/pull_request_creator/message_builder_spec.rb | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index 7155ea982fa..3ed53ce3396 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -75,9 +75,9 @@ def truncate_pr_message(msg) msg = msg.force_encoding(pr_message_encoding) unless pr_message_encoding.nil? if msg.length > pr_message_max_length - trunc_msg = pr_message_encoding.nil? ? TRUNCATED_MSG : (+TRUNCATED_MSG).dup.force_encoding(pr_message_encoding) - trunc_length = pr_message_max_length - trunc_msg.length - msg = (msg[0..trunc_length] + trunc_msg) + tr_msg = pr_message_encoding.nil? ? TRUNCATED_MSG : (+TRUNCATED_MSG).dup.force_encoding(pr_message_encoding) + trunc_length = pr_message_max_length - tr_msg.length + msg = (msg[0..trunc_length] + tr_msg) end msg.force_encoding(Encoding::UTF_8) unless pr_message_encoding.nil? msg diff --git a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb index ea22129b3be..e903ffbc7d1 100644 --- a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb +++ b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb @@ -2608,7 +2608,7 @@ def commits_details(base:, head:) it "truncates and maintains the specified encoding" do encode_utf16 = Encoding::UTF_16 - msg = message.dup.force_encoding + msg = message.dup.force_encoding(encode_utf16) trunc_msg = (+"...\n\n_Description has been truncated_").force_encoding(encode_utf16) trunc_length = pr_message_max_length - trunc_msg.length msg = "#{msg[0..trunc_length]}#{trunc_msg}" From a2c90e9cc78bb7ed6afc403b9ffc04d66ca1a86b Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 17:40:17 -0400 Subject: [PATCH 16/29] Remove useless ||= --- common/lib/dependabot/pull_request_creator.rb | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator.rb b/common/lib/dependabot/pull_request_creator.rb index 37c5c9450c1..b2d45b62ac8 100644 --- a/common/lib/dependabot/pull_request_creator.rb +++ b/common/lib/dependabot/pull_request_creator.rb @@ -229,21 +229,20 @@ def message pr_message_max_length = Codecommit::MAX_PR_DESCRIPTION_LENGTH if pr_message_max_length.nil? end - @message ||= - MessageBuilder.new( - source: source, - dependencies: dependencies, - files: files, - credentials: credentials, - commit_message_options: commit_message_options, - pr_message_header: pr_message_header, - pr_message_footer: pr_message_footer, - vulnerabilities_fixed: vulnerabilities_fixed, - github_redirection_service: github_redirection_service, - dependency_group: dependency_group, - pr_message_max_length: pr_message_max_length, - pr_message_encoding: pr_message_encoding - ) + @message = MessageBuilder.new( + source: source, + dependencies: dependencies, + files: files, + credentials: credentials, + commit_message_options: commit_message_options, + pr_message_header: pr_message_header, + pr_message_footer: pr_message_footer, + vulnerabilities_fixed: vulnerabilities_fixed, + github_redirection_service: github_redirection_service, + dependency_group: dependency_group, + pr_message_max_length: pr_message_max_length, + pr_message_encoding: pr_message_encoding + ) end def branch_namer From 3c9ddf55b24f583780d9915aca665dde70cebcdb Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 17:51:10 -0400 Subject: [PATCH 17/29] reference instance variables correctly and add missing pr_message_encoding attribute --- common/lib/dependabot/pull_request_creator.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator.rb b/common/lib/dependabot/pull_request_creator.rb index b2d45b62ac8..c32cc85a25e 100644 --- a/common/lib/dependabot/pull_request_creator.rb +++ b/common/lib/dependabot/pull_request_creator.rb @@ -49,7 +49,8 @@ def initialize(cause, pull_request) :commit_message_options, :vulnerabilities_fixed, :reviewers, :assignees, :milestone, :branch_name_separator, :branch_name_prefix, :branch_name_max_length, :github_redirection_service, - :custom_headers, :provider_metadata, :dependency_group, :pr_message_max_length + :custom_headers, :provider_metadata, :dependency_group, :pr_message_max_length, + :pr_message_encoding def initialize(source:, base_commit:, dependencies:, files:, credentials:, pr_message_header: nil, pr_message_footer: nil, @@ -61,7 +62,8 @@ def initialize(source:, base_commit:, dependencies:, files:, credentials:, automerge_candidate: false, github_redirection_service: DEFAULT_GITHUB_REDIRECTION_SERVICE, custom_headers: nil, require_up_to_date_base: false, - provider_metadata: {}, message: nil, dependency_group: nil, pr_message_max_length: nil) + provider_metadata: {}, message: nil, dependency_group: nil, pr_message_max_length: nil, + pr_message_encoding: nil) @dependencies = dependencies @source = source @base_commit = base_commit @@ -89,6 +91,7 @@ def initialize(source:, base_commit:, dependencies:, files:, credentials:, @message = message @dependency_group = dependency_group @pr_message_max_length = pr_message_max_length + @pr_message_encoding = pr_message_encoding check_dependencies_have_previous_version end @@ -221,12 +224,12 @@ def message case source.provider when "github" - pr_message_max_length = Github::MAX_PR_DESCRIPTION_LENGTH if pr_message_max_length.nil? + @pr_message_max_length = Github::MAX_PR_DESCRIPTION_LENGTH if @pr_message_max_length.nil? when "azure" - pr_message_max_length = Azure::MAX_PR_DESCRIPTION_LENGTH if pr_message_max_length.nil? - pr_message_encoding = Azure::ENCODING if pr_message_encoding.nil? + @pr_message_max_length = Azure::MAX_PR_DESCRIPTION_LENGTH if @pr_message_max_length.nil? + @pr_message_encoding = Azure::ENCODING if @pr_message_encoding.nil? when "codecommit" - pr_message_max_length = Codecommit::MAX_PR_DESCRIPTION_LENGTH if pr_message_max_length.nil? + @pr_message_max_length = Codecommit::MAX_PR_DESCRIPTION_LENGTH if @pr_message_max_length.nil? end @message = MessageBuilder.new( From 8aefb0b657c8789a6f066b49b2f3a3294931467f Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 18:13:18 -0400 Subject: [PATCH 18/29] Add return if message is not nil --- common/lib/dependabot/pull_request_creator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/dependabot/pull_request_creator.rb b/common/lib/dependabot/pull_request_creator.rb index c32cc85a25e..146f09584ee 100644 --- a/common/lib/dependabot/pull_request_creator.rb +++ b/common/lib/dependabot/pull_request_creator.rb @@ -220,7 +220,7 @@ def codecommit_creator end def message - @message unless @message.nil? + return @message unless @message.nil? case source.provider when "github" From 8d1c02df523d53ff1e02638a52fb71f2d3c78361 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 18:48:05 -0400 Subject: [PATCH 19/29] Reword comment --- .../lib/dependabot/pull_request_creator/message_builder.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index 3ed53ce3396..1c643f8a2c8 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -64,10 +64,7 @@ def pr_message suffixed_pr_message_header + prefixed_pr_message_footer end - # This method accepts a message, a max length and an optional encoding - # If the max length is not positive or the message length is less than max_length - # the original message is returned, otherwise it will optionally encode the message - # truncate it, then optional reencode back to the ruby standard of UTF_8 + # Truncate PR message as determined by the pr_message_max_length and pr_message_encoding instance variables def truncate_pr_message(msg) return msg if pr_message_max_length.nil? From e157faaaa43edb198de7ed5853b5dc54eb357b88 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Fri, 21 Jul 2023 18:49:43 -0400 Subject: [PATCH 20/29] add mention of why encoding is an option and what the return value will be --- common/lib/dependabot/pull_request_creator/message_builder.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index 1c643f8a2c8..e9a5725fc15 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -65,6 +65,7 @@ def pr_message end # Truncate PR message as determined by the pr_message_max_length and pr_message_encoding instance variables + # The encoding is used when calculating length, all messages are returned as ruby UTF_8 encoded string def truncate_pr_message(msg) return msg if pr_message_max_length.nil? From f89d72758e120512dda5ee828c32fbfdc8ced089 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Mon, 24 Jul 2023 10:28:21 -0400 Subject: [PATCH 21/29] Rename variables --- common/lib/dependabot/clients/azure.rb | 2 +- common/lib/dependabot/clients/codecommit.rb | 4 ++-- common/lib/dependabot/pull_request_creator.rb | 8 ++++---- common/lib/dependabot/pull_request_creator/azure.rb | 4 ++-- common/lib/dependabot/pull_request_creator/codecommit.rb | 2 +- common/lib/dependabot/pull_request_creator/github.rb | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/common/lib/dependabot/clients/azure.rb b/common/lib/dependabot/clients/azure.rb index 1da8540fc39..ed35c1ce15d 100644 --- a/common/lib/dependabot/clients/azure.rb +++ b/common/lib/dependabot/clients/azure.rb @@ -22,7 +22,7 @@ class TagsCreationForbidden < StandardError; end RETRYABLE_ERRORS = [InternalServerError, BadGateway, ServiceNotAvailable].freeze - MAX_PR_DESCRIPTION_LENGTH = 3999 + PR_DESCRIPTION_MAX_LENGTH = 3999 ####################### # Constructor methods # diff --git a/common/lib/dependabot/clients/codecommit.rb b/common/lib/dependabot/clients/codecommit.rb index fd5f92ecfca..18ec4c32df4 100644 --- a/common/lib/dependabot/clients/codecommit.rb +++ b/common/lib/dependabot/clients/codecommit.rb @@ -68,8 +68,8 @@ def fetch_file_contents(repo, commit, path) commit_specifier: commit, file_path: path ).file_content - rescue Aws::CodeCommit::Errors::FileDoesNotExistException - raise NotFound + rescue Aws::CodeCommit::Errors::FileDoesNotExistException + raise NotFound end def branch(branch_name) diff --git a/common/lib/dependabot/pull_request_creator.rb b/common/lib/dependabot/pull_request_creator.rb index 146f09584ee..df5c2cd8790 100644 --- a/common/lib/dependabot/pull_request_creator.rb +++ b/common/lib/dependabot/pull_request_creator.rb @@ -224,12 +224,12 @@ def message case source.provider when "github" - @pr_message_max_length = Github::MAX_PR_DESCRIPTION_LENGTH if @pr_message_max_length.nil? + @pr_message_max_length = Github::PR_DESCRIPTION_MAX_LENGTH if @pr_message_max_length.nil? when "azure" - @pr_message_max_length = Azure::MAX_PR_DESCRIPTION_LENGTH if @pr_message_max_length.nil? - @pr_message_encoding = Azure::ENCODING if @pr_message_encoding.nil? + @pr_message_max_length = Azure::PR_DESCRIPTION_MAX_LENGTH if @pr_message_max_length.nil? + @pr_message_encoding = Azure::PR_DESCRIPTION_ENCODING if @pr_message_encoding.nil? when "codecommit" - @pr_message_max_length = Codecommit::MAX_PR_DESCRIPTION_LENGTH if @pr_message_max_length.nil? + @pr_message_max_length = Codecommit::PR_DESCRIPTION_MAX_LENGTH if @pr_message_max_length.nil? end @message = MessageBuilder.new( diff --git a/common/lib/dependabot/pull_request_creator/azure.rb b/common/lib/dependabot/pull_request_creator/azure.rb index 4c1e9a14524..1cd81dbc498 100644 --- a/common/lib/dependabot/pull_request_creator/azure.rb +++ b/common/lib/dependabot/pull_request_creator/azure.rb @@ -10,8 +10,8 @@ class Azure :files, :commit_message, :pr_description, :pr_name, :author_details, :labeler, :reviewers, :assignees, :work_item - MAX_PR_DESCRIPTION_LENGTH = 3999 # Azure has a max length of 4000 (0 based count) - ENCODING = Encoding::UTF_16 # Azure DevOps only support descriptions up to 4000 characters in UTF-16 + PR_DESCRIPTION_MAX_LENGTH = 3999 # Azure has a max length of 4000 (0 based count) + PR_DESCRIPTION_ENCODING = Encoding::UTF_16 # Azure DevOps only support descriptions up to 4000 characters in UTF-16 # encoding. # https://developercommunity.visualstudio.com/content/problem/608770/remove-4000-character-limit-on-pull-request-descri.html diff --git a/common/lib/dependabot/pull_request_creator/codecommit.rb b/common/lib/dependabot/pull_request_creator/codecommit.rb index d542d61cdea..3fec858a4fd 100644 --- a/common/lib/dependabot/pull_request_creator/codecommit.rb +++ b/common/lib/dependabot/pull_request_creator/codecommit.rb @@ -10,7 +10,7 @@ class Codecommit :files, :commit_message, :pr_description, :pr_name, :author_details, :labeler - MAX_PR_DESCRIPTION_LENGTH = 10_239 # CodeCommit has a max length of 10240 (0 based count) + PR_DESCRIPTION_MAX_LENGTH = 10_239 # CodeCommit has a max length of 10240 (0 based count) # https://docs.aws.amazon.com/codecommit/latest/APIReference/API_PullRequest.html def initialize(source:, branch_name:, base_commit:, credentials:, diff --git a/common/lib/dependabot/pull_request_creator/github.rb b/common/lib/dependabot/pull_request_creator/github.rb index c58d85b5801..754d2264b9e 100644 --- a/common/lib/dependabot/pull_request_creator/github.rb +++ b/common/lib/dependabot/pull_request_creator/github.rb @@ -9,7 +9,7 @@ module Dependabot class PullRequestCreator # rubocop:disable Metrics/ClassLength class Github - MAX_PR_DESCRIPTION_LENGTH = 65_536 # Limit PR description to MAX_PR_DESCRIPTION_LENGTH (65,536) characters + PR_DESCRIPTION_MAX_LENGTH = 65_536 # Limit PR description to PR_DESCRIPTION_MAX_LENGTH (65,536) characters # and truncate with message if over. The API limit is 262,144 bytes # (https://github.community/t/maximum-length-for-the-comment-body-in-issues-and-pr/148867/2). # As Ruby strings are UTF-8 encoded, this is a pessimistic limit: it From b477cfae4bbe350db1f495f13ea69110db9952d4 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Mon, 24 Jul 2023 10:29:11 -0400 Subject: [PATCH 22/29] Add comment --- common/lib/dependabot/pull_request_creator/message_builder.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index e9a5725fc15..4c81ad31f44 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -77,6 +77,7 @@ def truncate_pr_message(msg) trunc_length = pr_message_max_length - tr_msg.length msg = (msg[0..trunc_length] + tr_msg) end + # if we used a custom encoding for calculating length, then we need to force back to UTF-8 msg.force_encoding(Encoding::UTF_8) unless pr_message_encoding.nil? msg end From dc2ba3a978cdb57fd440c040ca989db820e780c0 Mon Sep 17 00:00:00 2001 From: Jeff Widman Date: Tue, 25 Jul 2023 10:46:53 -0700 Subject: [PATCH 23/29] Update common/lib/dependabot/pull_request_creator/azure.rb --- common/lib/dependabot/pull_request_creator/azure.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/azure.rb b/common/lib/dependabot/pull_request_creator/azure.rb index 1cd81dbc498..e90f519cc09 100644 --- a/common/lib/dependabot/pull_request_creator/azure.rb +++ b/common/lib/dependabot/pull_request_creator/azure.rb @@ -10,10 +10,10 @@ class Azure :files, :commit_message, :pr_description, :pr_name, :author_details, :labeler, :reviewers, :assignees, :work_item - PR_DESCRIPTION_MAX_LENGTH = 3999 # Azure has a max length of 4000 (0 based count) - PR_DESCRIPTION_ENCODING = Encoding::UTF_16 # Azure DevOps only support descriptions up to 4000 characters in UTF-16 - # encoding. + # Azure DevOps limits PR descriptions to a max of 4,000 characters in UTF-16 encoding: # https://developercommunity.visualstudio.com/content/problem/608770/remove-4000-character-limit-on-pull-request-descri.html + PR_DESCRIPTION_MAX_LENGTH = 3_999 # 0 based count + PR_DESCRIPTION_ENCODING = Encoding::UTF_16 def initialize(source:, branch_name:, base_commit:, credentials:, files:, commit_message:, pr_description:, pr_name:, From 08ec1bceb12392a0fad1653750a9e50f3e859c97 Mon Sep 17 00:00:00 2001 From: Jeff Widman Date: Tue, 25 Jul 2023 10:46:59 -0700 Subject: [PATCH 24/29] Update common/lib/dependabot/pull_request_creator/github.rb --- common/lib/dependabot/pull_request_creator/github.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/github.rb b/common/lib/dependabot/pull_request_creator/github.rb index 754d2264b9e..3a9ab788051 100644 --- a/common/lib/dependabot/pull_request_creator/github.rb +++ b/common/lib/dependabot/pull_request_creator/github.rb @@ -9,11 +9,9 @@ module Dependabot class PullRequestCreator # rubocop:disable Metrics/ClassLength class Github - PR_DESCRIPTION_MAX_LENGTH = 65_536 # Limit PR description to PR_DESCRIPTION_MAX_LENGTH (65,536) characters - # and truncate with message if over. The API limit is 262,144 bytes - # (https://github.community/t/maximum-length-for-the-comment-body-in-issues-and-pr/148867/2). - # As Ruby strings are UTF-8 encoded, this is a pessimistic limit: it - # presumes the case where all characters are 4 bytes. + # GitHub limits PR descriptions to a max of 65,536 characters: + # https://github.com/orgs/community/discussions/27190#discussioncomment-3726017 + PR_DESCRIPTION_MAX_LENGTH = 65_536 attr_reader :source, :branch_name, :base_commit, :credentials, :files, :pr_description, :pr_name, :commit_message, From cb325b857215617cbd0818fa8fdd41acfe8ab12f Mon Sep 17 00:00:00 2001 From: Jeff Widman Date: Tue, 25 Jul 2023 10:47:06 -0700 Subject: [PATCH 25/29] Update common/lib/dependabot/pull_request_creator/codecommit.rb --- common/lib/dependabot/pull_request_creator/codecommit.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/codecommit.rb b/common/lib/dependabot/pull_request_creator/codecommit.rb index 3fec858a4fd..7b100475524 100644 --- a/common/lib/dependabot/pull_request_creator/codecommit.rb +++ b/common/lib/dependabot/pull_request_creator/codecommit.rb @@ -10,8 +10,9 @@ class Codecommit :files, :commit_message, :pr_description, :pr_name, :author_details, :labeler - PR_DESCRIPTION_MAX_LENGTH = 10_239 # CodeCommit has a max length of 10240 (0 based count) - # https://docs.aws.amazon.com/codecommit/latest/APIReference/API_PullRequest.html + # CodeCommit limits PR descriptions to a max length of 10,240 characters: + # https://docs.aws.amazon.com/codecommit/latest/APIReference/API_PullRequest.html + PR_DESCRIPTION_MAX_LENGTH = 10_239 # 0 based count def initialize(source:, branch_name:, base_commit:, credentials:, files:, commit_message:, pr_description:, pr_name:, From 02259145464779450c86de2cf340dcaa19ada505 Mon Sep 17 00:00:00 2001 From: Jeff Widman Date: Tue, 25 Jul 2023 10:51:16 -0700 Subject: [PATCH 26/29] Update common/lib/dependabot/pull_request_creator/github.rb --- common/lib/dependabot/pull_request_creator/github.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/dependabot/pull_request_creator/github.rb b/common/lib/dependabot/pull_request_creator/github.rb index 3a9ab788051..e460aa6e590 100644 --- a/common/lib/dependabot/pull_request_creator/github.rb +++ b/common/lib/dependabot/pull_request_creator/github.rb @@ -11,7 +11,7 @@ class PullRequestCreator class Github # GitHub limits PR descriptions to a max of 65,536 characters: # https://github.com/orgs/community/discussions/27190#discussioncomment-3726017 - PR_DESCRIPTION_MAX_LENGTH = 65_536 + PR_DESCRIPTION_MAX_LENGTH = 65_535 # 0 based count attr_reader :source, :branch_name, :base_commit, :credentials, :files, :pr_description, :pr_name, :commit_message, From 36e068733f74393b441ac59850eaebab835ca188 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Tue, 25 Jul 2023 13:52:08 -0400 Subject: [PATCH 27/29] Remove old variable that is now in pull_request_creator --- common/lib/dependabot/clients/azure.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/lib/dependabot/clients/azure.rb b/common/lib/dependabot/clients/azure.rb index ed35c1ce15d..27bc596b8d9 100644 --- a/common/lib/dependabot/clients/azure.rb +++ b/common/lib/dependabot/clients/azure.rb @@ -22,8 +22,6 @@ class TagsCreationForbidden < StandardError; end RETRYABLE_ERRORS = [InternalServerError, BadGateway, ServiceNotAvailable].freeze - PR_DESCRIPTION_MAX_LENGTH = 3999 - ####################### # Constructor methods # ####################### From 0d82621e6ae8d22878582f6a8199ea6f6644fade Mon Sep 17 00:00:00 2001 From: Jeff Widman Date: Tue, 25 Jul 2023 10:58:04 -0700 Subject: [PATCH 28/29] Update common/lib/dependabot/pull_request_creator/codecommit.rb --- common/lib/dependabot/pull_request_creator/codecommit.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/codecommit.rb b/common/lib/dependabot/pull_request_creator/codecommit.rb index 7b100475524..1ea54f3191e 100644 --- a/common/lib/dependabot/pull_request_creator/codecommit.rb +++ b/common/lib/dependabot/pull_request_creator/codecommit.rb @@ -10,8 +10,8 @@ class Codecommit :files, :commit_message, :pr_description, :pr_name, :author_details, :labeler - # CodeCommit limits PR descriptions to a max length of 10,240 characters: - # https://docs.aws.amazon.com/codecommit/latest/APIReference/API_PullRequest.html + # CodeCommit limits PR descriptions to a max length of 10,240 characters: + # https://docs.aws.amazon.com/codecommit/latest/APIReference/API_PullRequest.html PR_DESCRIPTION_MAX_LENGTH = 10_239 # 0 based count def initialize(source:, branch_name:, base_commit:, credentials:, From dbfd0bd073ed43e40e390d1c0ecc4b2d954b3dcc Mon Sep 17 00:00:00 2001 From: Jeff Widman Date: Tue, 25 Jul 2023 10:58:33 -0700 Subject: [PATCH 29/29] Update common/lib/dependabot/pull_request_creator/azure.rb --- common/lib/dependabot/pull_request_creator/azure.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/dependabot/pull_request_creator/azure.rb b/common/lib/dependabot/pull_request_creator/azure.rb index e90f519cc09..0c605faa45e 100644 --- a/common/lib/dependabot/pull_request_creator/azure.rb +++ b/common/lib/dependabot/pull_request_creator/azure.rb @@ -13,7 +13,7 @@ class Azure # Azure DevOps limits PR descriptions to a max of 4,000 characters in UTF-16 encoding: # https://developercommunity.visualstudio.com/content/problem/608770/remove-4000-character-limit-on-pull-request-descri.html PR_DESCRIPTION_MAX_LENGTH = 3_999 # 0 based count - PR_DESCRIPTION_ENCODING = Encoding::UTF_16 + PR_DESCRIPTION_ENCODING = Encoding::UTF_16 def initialize(source:, branch_name:, base_commit:, credentials:, files:, commit_message:, pr_description:, pr_name:,