removes scheduled state transition and updates sync logic

app/models/remote_mirror.rb

@@ -29,16 +29,8 @@ class RemoteMirror < ActiveRecord::Base
   scope :stuck,   -> { started.where('last_update_at < ? OR (last_update_at IS NULL AND updated_at < ?)', 1.day.ago, 1.day.ago) }
 
   state_machine :update_status, initial: :none do
-    event :schedule do
-      transition [:none, :finished] => :scheduling
-    end
-
-    event :reschedule do
-      transition failed: :scheduling
-    end
-
     event :update_start do
-      transition scheduling: :started
+      transition [:none, :finished, :failed] => :started
     end
 
     event :update_finish do
@@ -49,14 +41,11 @@ class RemoteMirror < ActiveRecord::Base
       transition started: :failed
     end
 
-    state :scheduling
     state :started
     state :finished
     state :failed
 
-    after_transition any => :scheduling, do: :schedule_update_job
-
-    after_transition scheduling: :started do |remote_mirror, _|
+    after_transition any => :started do |remote_mirror, _|
       remote_mirror.update(last_update_started_at: Time.now)
     end
 
@@ -88,7 +77,7 @@ class RemoteMirror < ActiveRecord::Base
     return unless project
     return if !enabled || update_in_progress?
 
-    update_failed? ? reschedule : schedule
+    schedule_update_job
   end
 
   def updated_since?(timestamp)
@@ -146,10 +135,6 @@ class RemoteMirror < ActiveRecord::Base
   end
 
   def schedule_update_job
-    run_after_commit(:add_update_job)
-  end
-
-  def add_update_job
     RepositoryUpdateRemoteMirrorWorker.perform_in(BACKOFF_DELAY, self.id, Time.now) if project&.repository_exists?
   end
 

spec/models/remote_mirror_spec.rb

@@ -85,6 +85,34 @@ describe RemoteMirror do
     end
   end
 
+  context '#updated_since?' do
+    let(:remote_mirror) { create(:project, :remote_mirror).remote_mirrors.first }
+    let(:timestamp) { Time.now - 5.minutes }
+
+    before do
+      Timecop.freeze(Time.now)
+      remote_mirror.update_attributes(last_update_started_at: Time.now)
+    end
+
+    context 'when remote mirror does not have status failed' do
+      it 'returns true when last update started after the timestamp' do
+        expect(remote_mirror.updated_since?(timestamp)).to be true
+      end
+
+      it 'returns false when last update started before the timestamp' do
+        expect(remote_mirror.updated_since?(Time.now + 5.minutes)).to be  false
+      end
+    end
+
+    context 'when remote mirror has status failed' do
+      it 'returns false when last update started after the timestamp' do
+        remote_mirror.update_attributes(update_status: 'failed')
+
+        expect(remote_mirror.updated_since?(timestamp)).to be false
+      end
+    end
+  end
+
   context 'no project' do
     it 'includes mirror with a project in pending_delete' do
       mirror = create_mirror(url: 'http://cantbeblank',

spec/workers/repository_update_remote_mirror_worker_spec.rb

@@ -4,29 +4,56 @@ describe RepositoryUpdateRemoteMirrorWorker do
   subject { described_class.new }
 
   let(:remote_mirror) { create(:project, :remote_mirror).remote_mirrors.first }
+  let(:scheduled_time) { Time.now - 5.minutes }
 
   before do
-    remote_mirror.update_attributes(update_status: 'started')
+    Timecop.freeze(Time.now)
   end
 
   describe '#perform' do
-    it 'sets sync as finished when update remote mirror service executes successfully' do
+    context 'with status scheduling' do
+      before do
+        remote_mirror.update_attributes(update_status: 'none')
+      end
+
+      it 'sets status as finished when update remote mirror service executes successfully' do
         expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :success)
 
         expect { subject.perform(remote_mirror.id, Time.now) }.to change { remote_mirror.reload.update_status }.to('finished')
       end
 
-    it 'sets sync as failed when update remote mirror service executes with errors' do
+      it 'sets status as failed when update remote mirror service executes with errors' do
         expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :error, message: 'fail!')
 
-      expect { subject.perform(remote_mirror.id, Time.now) }.to change { remote_mirror.reload.update_status }.to('failed')
+        expect do
+          subject.perform(remote_mirror.id, Time.now)
+        end.to raise_error
+        expect(remote_mirror.reload.update_status).to eq('failed')
       end
 
-    it 'does nothing if last_update_at is higher than the time the job was scheduled in' do
-      expect_any_instance_of(RemoteMirror).to receive(:last_update_at).and_return(Time.now + 10.minutes)
+      it 'does nothing if last_update_started_at is higher than the time the job was scheduled in' do
+        remote_mirror.update_attributes(last_update_started_at: Time.now)
+
+        expect_any_instance_of(RemoteMirror).to receive(:updated_since?).with(scheduled_time).and_return(true)
         expect_any_instance_of(Projects::UpdateRemoteMirrorService).not_to receive(:execute).with(remote_mirror)
 
-      expect(subject.perform(remote_mirror.id, Time.now)).to be_nil
+        expect(subject.perform(remote_mirror.id, scheduled_time)).to be_nil
+      end
+    end
+
+    context 'with status failed' do
+      before do
+        remote_mirror.update_attributes(update_status: 'failed')
+      end
+
+      it 'sets status as finished if last_update_started_at is higher than the time the job was scheduled in' do
+        remote_mirror.update_attributes(last_update_started_at: Time.now)
+
+        expect_any_instance_of(RemoteMirror).to receive(:updated_since?).with(scheduled_time).and_return(false)
+        expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :success)
+
+        expect { subject.perform(remote_mirror.id, scheduled_time) }.to change { remote_mirror.reload.update_status }.to('finished')
+      end
     end
   end
 end