diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb index 2fd6ea877e334249d6629cf748126a0a68b6ee0b..a4f7113519ebb49f2088d95b0a86fb141b249769 100644 --- a/app/models/remote_mirror.rb +++ b/app/models/remote_mirror.rb @@ -39,9 +39,15 @@ class RemoteMirror < ActiveRecord::Base transition failed: :started end - state :started + state :started do + validates_presence_of :project + end + state :finished - state :failed + + state :failed do + validates_presence_of :project + end after_transition any => :started, do: :schedule_update_job @@ -80,8 +86,9 @@ def mark_for_delete_if_blank_url end def mark_as_failed(error_message) - update_fail - update_column(:last_error, Gitlab::UrlSanitizer.sanitize(error_message)) + error_message = errors.full_messages.first unless update_fail + + update_columns(last_error: Gitlab::UrlSanitizer.sanitize(error_message), update_status: 'failed') end def url=(value) @@ -109,7 +116,7 @@ def safe_url private def url_availability - if project.import_url == url && project.mirror? + if project && project.import_url == url && project.mirror? errors.add(:url, 'is already in use') end end diff --git a/app/workers/repository_update_remote_mirror_worker.rb b/app/workers/repository_update_remote_mirror_worker.rb index fdcae2895ef2f79c7f3dc7130beb5d98d9d1af7c..3993788fb698ac65f2594f81b215b1e23a6230cf 100644 --- a/app/workers/repository_update_remote_mirror_worker.rb +++ b/app/workers/repository_update_remote_mirror_worker.rb @@ -10,6 +10,9 @@ def perform(remote_mirror_id) begin remote_mirror = RemoteMirror.find(remote_mirror_id) project = remote_mirror.project + + return unless project + current_user = project.creator result = Projects::UpdateRemoteMirrorService.new(project, current_user).execute(remote_mirror) diff --git a/changelogs/unreleased-ee/1165-jej-remote-mirror-validation-failures.yml b/changelogs/unreleased-ee/1165-jej-remote-mirror-validation-failures.yml new file mode 100644 index 0000000000000000000000000000000000000000..9d9302ca1a0b5ef55695f2e261e0bf0b4b91fd62 --- /dev/null +++ b/changelogs/unreleased-ee/1165-jej-remote-mirror-validation-failures.yml @@ -0,0 +1,4 @@ +--- +title: Fix UpdateAllRemoteMirrorsWorker failing when remote mirror has no project +merge_request: +author: diff --git a/spec/factories/remote_mirrors.rb b/spec/factories/remote_mirrors.rb new file mode 100644 index 0000000000000000000000000000000000000000..1e534325d8d07498ae1b4403d1963c552bd4150c --- /dev/null +++ b/spec/factories/remote_mirrors.rb @@ -0,0 +1,6 @@ +FactoryGirl.define do + factory :remote_mirror do + url 'http://cantbeblank' + project + end +end diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 46bc8affe6498c72bf38c30c68d9592c24c247b6..63241aa111780df71a8e9d112d265fe8841a86de 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -84,6 +84,24 @@ end end + context 'with missing project' do + let(:mirror) { create(:remote_mirror, update_status: 'started') } + + before do + mirror.update_columns(enabled: true, project_id: nil) + + mirror.reload.mark_as_failed('missing project') + end + + it 'sets the last error' do + expect(mirror.reload.last_error).to include("Project can't be blank") + end + + it 'is marked as failed' do + expect(mirror.reload.update_status).to eq('failed') + end + end + def create_mirror(params) project = FactoryGirl.create(:project) project.remote_mirrors.create!(params) diff --git a/spec/workers/repository_update_remote_mirror_worker_spec.rb b/spec/workers/repository_update_remote_mirror_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..46d1a2c96cc29bfb47102551d137c31705c3b18c --- /dev/null +++ b/spec/workers/repository_update_remote_mirror_worker_spec.rb @@ -0,0 +1,13 @@ +require 'rails_helper' + +describe RepositoryUpdateRemoteMirrorWorker do + let(:mirror) { create(:remote_mirror) } + + describe '#perform' do + it 'does nothing if project is missing' do + mirror.update!(project_id: nil) + + expect(described_class.new.perform(mirror.id)).to be_nil + end + end +end diff --git a/spec/workers/update_all_remote_mirrors_worker_spec.rb b/spec/workers/update_all_remote_mirrors_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..3266ee93b568ceef8488bca99643dacb538c0750 --- /dev/null +++ b/spec/workers/update_all_remote_mirrors_worker_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' + +describe UpdateAllRemoteMirrorsWorker do + let(:worker) { described_class.new } + + describe '#perform' do + context 'stuck mirrors' do + let!(:mirror) { create(:remote_mirror, update_status: :started, last_update_at: 1.week.ago) } + + it 'are transitioned to failure state' do + worker.perform + + expect(mirror.reload.update_status).to eq('failed') + end + + it 'handles enabled mirrors with missing project' do + mirror.update_attributes(project_id: nil, enabled: true) + + worker.perform + + expect(mirror.reload.update_status).to eq('failed') + end + end + end +end