Introduce feature flag to raise exception on same branch exists (#10878)

Introduce feature flag to raise exception on same branch exists (#10878)

common/lib/dependabot/pull_request_creator.rb

@@ -40,6 +40,8 @@ class NoHistoryInCommon < StandardError; end
 
     class UnmergedPRExists < StandardError; end
 
+    class BranchAlreadyExists < StandardError; end
+
     class BaseCommitNotUpToDate < StandardError; end
 
     class UnexpectedError < StandardError; end
@@ -396,12 +398,6 @@ def message
 
     sig { returns(Dependabot::PullRequestCreator::BranchNamer) }
     def branch_namer
-      if Dependabot::Experiments.enabled?(:dedup_branch_names) && existing_branches
-        Dependabot.logger.debug(
-          "Dependabot::PullRequestCreator::branch_namer : #{existing_branches}"
-        )
-      end
-
       @branch_namer ||= T.let(
         BranchNamer.new(
           dependencies: dependencies,

common/lib/dependabot/pull_request_creator/branch_namer/base.rb

@@ -74,32 +74,7 @@ def sanitize_branch_name(ref_name)
             sanitized_name[[T.must(max_length) - sha.size, 0].max..] = sha
           end
 
-          if Dependabot::Experiments.enabled?(:dedup_branch_names)
-            dedup_existing_branches(sanitized_name)
-          else
-            sanitized_name
-          end
-        end
-
-        sig { params(ref: String).returns(String) }
-        def dedup_existing_branches(ref)
-          Dependabot.logger.debug(
-            "Dependabot::PullRequestCreator::dedup_existing_branches::ref : #{ref}"
-          )
-          return ref unless existing_branches.include?(ref)
-
-          i = 1
-          new_ref = "#{ref}-#{i}"
-          while existing_branches.include?(new_ref)
-            i += 1
-            new_ref = "#{ref}-#{i}"
-          end
-
-          Dependabot.logger.debug(
-            "Dependabot::PullRequestCreator::dedup_existing_branches::new_ref : #{new_ref}"
-          )
-
-          new_ref
+          sanitized_name
         end
 
         sig { params(ref: String).returns(String) }

common/lib/dependabot/pull_request_creator/github.rb

@@ -110,6 +110,13 @@ def initialize(source:, branch_name:, base_commit:, credentials:,
 
       sig { returns(T.untyped) }
       def create
+        if experiment_duplicate_branch? && branch_exists?(branch_name)
+          Dependabot.logger.info(
+            "Existing branch \"#{branch_name}\" found. Pull request not created."
+          )
+          raise BranchAlreadyExists, "Duplicate branch #{branch_name} already exists"
+        end
+
         if branch_exists?(branch_name) && unmerged_pull_request_exists?
           raise UnmergedPRExists, "PR ##{unmerged_pull_requests.first.number} already exists"
         end
@@ -132,6 +139,11 @@ def require_up_to_date_base?
       # rubocop:disable Metrics/PerceivedComplexity
       sig { params(name: String).returns(T::Boolean) }
       def branch_exists?(name)
+        Dependabot.logger.debug(
+          "Dependabot::PullRequestCreator::Github:branch_exists?. " \
+          "Name : #{name}. IsDuplicate: #{git_metadata_fetcher.ref_names.include?(name)}"
+        )
+
         git_metadata_fetcher.ref_names.include?(name)
       rescue Dependabot::GitDependenciesNotReachable => e
         raise T.must(e.cause) if e.cause&.message&.include?("is disabled")
@@ -580,6 +592,11 @@ def raise_custom_error(base_err, type, message)
           raise type, message
         end
       end
+
+      sig { returns(T::Boolean) }
+      def experiment_duplicate_branch?
+        Dependabot::Experiments.enabled?(:dedup_branch_names)
+      end
     end
     # rubocop:enable Metrics/ClassLength
   end

common/spec/dependabot/pull_request_creator/branch_namer_spec.rb

@@ -88,29 +88,6 @@
       it { is_expected.to eq("myapp/dummy/business-1.5.0") }
     end
 
-    context "with a branch name that already exists" do
-      before do
-        Dependabot::Experiments.register(:dedup_branch_names, true)
-      end
-
-      let!(:existing_branches) do
-        [
-          "dependabot/dummy/business-1.5.0",
-          "dependabot/dummy/business-1.5.0-1"
-        ]
-      end
-      let(:namer) do
-        described_class.new(
-          dependencies: dependencies,
-          files: files,
-          target_branch: target_branch,
-          existing_branches: existing_branches
-        )
-      end
-
-      it { is_expected.to eq("dependabot/dummy/business-1.5.0-2") }
-    end
-
     context "with a target branch" do
       let(:target_branch) { "my-branch" }
 

common/spec/dependabot/pull_request_creator/github_spec.rb

@@ -612,6 +612,41 @@
       end
     end
 
+    context "when the branch already exists" do
+      let(:service_pack_response) { fixture("git", "upload_packs", "existing-branch") }
+
+      before do
+        Dependabot::Experiments.register(:dedup_branch_names, true)
+      end
+
+      after do
+        Dependabot::Experiments.register(:dedup_branch_names, false)
+      end
+
+      context "when the branch already exists" do
+        before do
+          url = "#{repo_api_url}/pulls?head=gocardless:#{branch_name}" \
+                "&state=all"
+          stub_request(:get, url)
+            .to_return(status: 200, body: "[]", headers: json_header)
+          stub_request(
+            :patch,
+            "#{repo_api_url}/git/refs/heads/#{branch_name}"
+          ).to_return(
+            status: 200,
+            body: fixture("github", "update_ref.json"),
+            headers: json_header
+          )
+        end
+
+        it "returns a suitable exception" do
+          expect { creator.create }
+            .to raise_error(Dependabot::PullRequestCreator::BranchAlreadyExists,
+                            "Duplicate branch #{branch_name} already exists")
+        end
+      end
+    end
+
     context "when the PR doesn't have history in common with the base branch" do
       before do
         stub_request(:post, "#{repo_api_url}/pulls")