Commit 429ae9ad authored by Paco Guzman's avatar Paco Guzman

Fix projects with remote mirrors asynchronously destruction (using pending_delete)

When we update the flag pending_delete to true and later try to 
destroy the remote mirror (through association callbacks) the remote mirror try to load the project, but it is out of the default scope so it just return nil.

As we need the project to remove the reference to the remote mirror but we’re going to delete the project and its repository is not needed to touch the repository to just remove that reference, so we just skip that step in that case
parent 14aeb8db
...@@ -8,6 +8,7 @@ v 8.12.0 (Unreleased) ...@@ -8,6 +8,7 @@ v 8.12.0 (Unreleased)
- [ES] Fix: Elasticsearch does not find partial matches in project names - [ES] Fix: Elasticsearch does not find partial matches in project names
- [ES] Global code search - [ES] Global code search
- [ES] Improve logging - [ES] Improve logging
- Fix projects with remote mirrors asynchronously destruction
v 8.11.6 v 8.11.6
- Exclude blocked users from potential MR approvers. - Exclude blocked users from potential MR approvers.
......
...@@ -158,8 +158,10 @@ class RemoteMirror < ActiveRecord::Base ...@@ -158,8 +158,10 @@ class RemoteMirror < ActiveRecord::Base
end end
def remove_remote def remove_remote
if project # could be pending to delete so don't need to touch the git repository
project.repository.remove_remote(ref_name) project.repository.remove_remote(ref_name)
end end
end
def mirror_url_changed? def mirror_url_changed?
url_changed? || encrypted_credentials_changed? url_changed? || encrypted_credentials_changed?
......
...@@ -5,6 +5,7 @@ describe Projects::DestroyService, services: true do ...@@ -5,6 +5,7 @@ describe Projects::DestroyService, services: true do
let!(:project) { create(:project, namespace: user.namespace) } let!(:project) { create(:project, namespace: user.namespace) }
let!(:path) { project.repository.path_to_repo } let!(:path) { project.repository.path_to_repo }
let!(:remove_path) { path.sub(/\.git\Z/, "+#{project.id}+deleted.git") } let!(:remove_path) { path.sub(/\.git\Z/, "+#{project.id}+deleted.git") }
let!(:async) { false } # execute or async_execute
context 'Sidekiq inline' do context 'Sidekiq inline' do
before do before do
...@@ -15,6 +16,19 @@ describe Projects::DestroyService, services: true do ...@@ -15,6 +16,19 @@ describe Projects::DestroyService, services: true do
it { expect(Project.all).not_to include(project) } it { expect(Project.all).not_to include(project) }
it { expect(Dir.exist?(path)).to be_falsey } it { expect(Dir.exist?(path)).to be_falsey }
it { expect(Dir.exist?(remove_path)).to be_falsey } it { expect(Dir.exist?(remove_path)).to be_falsey }
context 'when has remote mirrors' do
let!(:project) do
create(:project, namespace: user.namespace).tap do |project|
project.remote_mirrors.create(url: 'http://test.com')
end
end
let!(:async) { true }
it 'destroys them' do
expect(RemoteMirror.count).to eq(0)
end
end
end end
context 'Sidekiq fake' do context 'Sidekiq fake' do
...@@ -52,6 +66,10 @@ describe Projects::DestroyService, services: true do ...@@ -52,6 +66,10 @@ describe Projects::DestroyService, services: true do
end end
def destroy_project(project, user, params) def destroy_project(project, user, params)
if async
Projects::DestroyService.new(project, user, params).async_execute
else
Projects::DestroyService.new(project, user, params).execute Projects::DestroyService.new(project, user, params).execute
end end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment