Merge pull request #22972 from agrare/delete_git_repo_dir_if_url_changed

Update GitWorktree/Rugged remote URL before pull if it has changed

app/models/git_repository.rb

@@ -114,6 +114,10 @@ def update_repo
     with_worktree do |worktree|
       message = "Updating #{url} in #{directory_name}..."
       _log.info(message)
+      # If the remote url has changed set it to the new url
+      git_transaction { worktree.url = url } if worktree.url != url
+
+      # Fetch and rebase the git worktree
       worktree.pull
       _log.info("#{message}...Complete")
     end

lib/git_worktree.rb

@@ -60,6 +60,14 @@ def delete_repo
     true
   end
 
+  def url
+    @repo.remotes[@remote_name]&.url
+  end
+
+  def url=(url)
+    lock { @repo.remotes.set_url(@remote_name, url) }
+  end
+
   def branches(where = nil)
     where.nil? ? @repo.branches.each_name.sort : @repo.branches.each_name(where).sort
   end

spec/models/git_repository_spec.rb

@@ -72,6 +72,7 @@
 
       before do
         EvmSpecHelper.local_miq_server
+        allow(gwt).to receive(:url).and_return(repo.url)
         allow(gwt).to receive(:branches).with(anything).and_return(branch_list)
         allow(gwt).to receive(:tags).with(no_args).and_return(tag_list)
         allow(gwt).to receive(:branch_info) do |name|
@@ -127,23 +128,26 @@
         repo.refresh
       end
 
-      it "doesn't send the proxy settings if the repo scheme is not http or https" do
-        proxy_settings = {
-          :git_repository_proxy => {
-            :host   => "example.com",
-            :port   => "3128",
-            :scheme => "http"
+      context "if the repo scheme is not http or https" do
+        let(:repo) { FactoryBot.create(:git_repository, :verify_ssl => verify_ssl, :url => "git@example.com:ManageIQ/manageiq.git") }
+
+        it "doesn't send the proxy settings" do
+          proxy_settings = {
+            :git_repository_proxy => {
+              :host   => "example.com",
+              :port   => "3128",
+              :scheme => "http"
+            }
           }
-        }
-        stub_settings(proxy_settings)
-        expect(GitWorktree).to receive(:new) do |options|
-          expect(options[:proxy_url]).to be_nil
-        end.twice.and_return(gwt)
+          stub_settings(proxy_settings)
+          expect(GitWorktree).to receive(:new) do |options|
+            expect(options[:proxy_url]).to be_nil
+          end.twice.and_return(gwt)
 
-        expect(gwt).to receive(:pull).with(no_args)
+          expect(gwt).to receive(:pull).with(no_args)
 
-        repo.update!(:url => "git@example.com:ManageIQ/manageiq.git")
-        repo.refresh
+          repo.refresh
+        end
       end
 
       context "self signed certifcate" do
@@ -161,28 +165,44 @@
       end
     end
 
-    it "#refresh" do
-      expect(GitWorktree).to receive(:new).with(anything).and_return(gwt)
-      expect(gwt).to receive(:branches).with(anything).and_return(branch_list)
-      expect(gwt).to receive(:tags).with(no_args).and_return(tag_list)
-      allow(gwt).to receive(:branch_info) do |name|
-        branch_info_hash[name]
+    describe "#refresh" do
+      before do
+        expect(GitWorktree).to receive(:new).twice.with(anything).and_return(gwt)
+        expect(gwt).to receive(:url).and_return(repo.url)
+        expect(gwt).to receive(:branches).with(anything).and_return(branch_list)
+        expect(gwt).to receive(:tags).with(no_args).and_return(tag_list)
+        allow(gwt).to receive(:branch_info) do |name|
+          branch_info_hash[name]
+        end
+        allow(gwt).to receive(:tag_info) do |name|
+          tag_info_hash[name]
+        end
+        expect(repo).to receive(:clone_repo_if_missing).once.with(no_args).and_call_original
+        expect(gwt).to receive(:pull).with(no_args)
       end
-      allow(gwt).to receive(:tag_info) do |name|
-        tag_info_hash[name]
+
+      it "fetches git branches and tags" do
+        repo.refresh
+        expect(repo.git_branches.collect(&:name)).to match_array(branch_list)
+        expect(repo.git_tags.collect(&:name)).to match_array(tag_list)
       end
 
-      expect(repo).to receive(:clone_repo_if_missing).once.with(no_args).and_call_original
-      expect(GitWorktree).to receive(:new).with(anything).and_return(gwt)
-      expect(gwt).to receive(:pull).with(no_args)
+      context "with a different URL from the git_worktree" do
+        let(:new_url) { "git@github.com:ManageIQ/manageiq.git" }
+        it "updates the new url before pull" do
+          repo.update!(:url => new_url)
+          expect(gwt).to receive(:url=).with(new_url)
 
-      repo.refresh
-      expect(repo.git_branches.collect(&:name)).to match_array(branch_list)
-      expect(repo.git_tags.collect(&:name)).to match_array(tag_list)
+          repo.refresh
+          expect(repo.git_branches.collect(&:name)).to match_array(branch_list)
+          expect(repo.git_tags.collect(&:name)).to match_array(tag_list)
+        end
+      end
     end
 
     it "#branch_info" do
       expect(GitWorktree).to receive(:new).twice.with(anything).and_return(gwt)
+      expect(gwt).to receive(:url).and_return(repo.url)
       expect(gwt).to receive(:branches).with(anything).and_return(branch_list)
       expect(gwt).to receive(:tags).with(no_args).and_return(tag_list)
       expect(gwt).to receive(:pull).with(no_args)
@@ -200,6 +220,7 @@
 
     it "#tag_info" do
       expect(GitWorktree).to receive(:new).twice.with(anything).and_return(gwt)
+      expect(gwt).to receive(:url).and_return(repo.url)
       expect(gwt).to receive(:branches).with(anything).and_return(branch_list)
       expect(gwt).to receive(:tags).with(no_args).and_return(tag_list)
       expect(gwt).to receive(:pull).with(no_args)
@@ -217,6 +238,7 @@
 
     it "#tag_info missing tag" do
       expect(GitWorktree).to receive(:new).twice.with(anything).and_return(gwt)
+      expect(gwt).to receive(:url).and_return(repo.url)
       expect(gwt).to receive(:branches).with(anything).and_return(branch_list)
       expect(gwt).to receive(:tags).with(no_args).and_return(tag_list)
       expect(gwt).to receive(:pull).with(no_args)
@@ -233,6 +255,7 @@
 
     it "#branch_info missing branch" do
       expect(GitWorktree).to receive(:new).twice.with(anything).and_return(gwt)
+      expect(gwt).to receive(:url).and_return(repo.url)
       expect(gwt).to receive(:branches).with(anything).and_return(branch_list)
       expect(gwt).to receive(:tags).with(no_args).and_return(tag_list)
       expect(gwt).to receive(:pull).with(no_args)
@@ -249,6 +272,7 @@
 
     it "#refresh branches deleted" do
       expect(GitWorktree).to receive(:new).twice.with(anything).and_return(gwt)
+      expect(gwt).to receive(:url).twice.and_return(repo.url)
       expect(gwt).to receive(:pull).twice.with(no_args)
       expect(gwt).to receive(:branches).twice.with(anything).and_return(branch_list)
       expect(gwt).to receive(:tags).twice.with(no_args).and_return(tag_list)
@@ -269,6 +293,7 @@
 
     it "#refresh tags deleted" do
       expect(GitWorktree).to receive(:new).twice.with(anything).and_return(gwt)
+      expect(gwt).to receive(:url).twice.and_return(repo.url)
       expect(gwt).to receive(:pull).twice.with(no_args)
       expect(gwt).to receive(:branches).twice.with(anything).and_return(branch_list)
       expect(gwt).to receive(:tags).twice.with(no_args).and_return(tag_list)