Add validator

5 files changed, 34 insertions, 41 deletions

diff --git a/app/models/ci/trigger_schedule.rb b/app/models/ci/trigger_schedule.rb
index aedba8bdf54..6529e364fe8 100644
--- a/app/models/ci/trigger_schedule.rb
+++ b/app/models/ci/trigger_schedule.rb
@@ -10,10 +10,10 @@ module Ci
     delegate :ref, to: :trigger
 
     validates :trigger, presence: true
-    validates :cron, presence: true
+    validates :cron, cron: true, presence: true
     validates :cron_time_zone, presence: true
-    validate :check_cron
-    validate :check_ref
+    validates :ref, ref: true, presence: true
+    validate :check_cron_frequency
 
     after_create :schedule_next_run!
 
@@ -31,26 +31,12 @@ module Ci
       ((time - Time.now).abs < 1.hour) ? true : false
     end
 
-    def check_cron
-      cron_parser = Ci::CronParser.new(cron, cron_time_zone)
-      is_valid_cron, is_valid_cron_time_zone = cron_parser.validation
-      next_time = cron_parser.next_time_from(Time.now)
+    def check_cron_frequency
+      next_time = Ci::CronParser.new(cron, cron_time_zone).next_time_from(Time.now)
 
-      if !is_valid_cron
-        self.errors.add(:cron, " is invalid syntax")
-      elsif !is_valid_cron_time_zone
-        self.errors.add(:cron_time_zone, " is invalid timezone")
-      elsif less_than_1_hour_from_now?(next_time)
+      if less_than_1_hour_from_now?(next_time)
         self.errors.add(:cron, " can not be less than 1 hour")
       end
     end
-
-    def check_ref
-      if !ref.present?
-        self.errors.add(:ref, " is empty")
-      elsif !project.repository.branch_exists?(ref)
-        self.errors.add(:ref, " does not exist")
-      end
-    end
   end
 end
diff --git a/app/validators/cron_validator.rb b/app/validators/cron_validator.rb
new file mode 100644
index 00000000000..ad70e0897ba
--- /dev/null
+++ b/app/validators/cron_validator.rb
@@ -0,0 +1,16 @@
+# CronValidator
+#
+# Custom validator for Cron.
+class CronValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    cron_parser = Ci::CronParser.new(record.cron, record.cron_time_zone)
+    is_valid_cron, is_valid_cron_time_zone = cron_parser.validation
+    next_time = cron_parser.next_time_from(Time.now)
+
+    if !is_valid_cron
+      record.errors.add(:cron, " is invalid syntax")
+    elsif !is_valid_cron_time_zone
+      record.errors.add(:cron_time_zone, " is invalid timezone")
+    end
+  end
+end
diff --git a/app/validators/ref_validator.rb b/app/validators/ref_validator.rb
new file mode 100644
index 00000000000..2024255a770
--- /dev/null
+++ b/app/validators/ref_validator.rb
@@ -0,0 +1,10 @@
+# RefValidator
+#
+# Custom validator for Ref.
+class RefValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    unless record.project.repository.branch_exists?(value)
+      record.errors.add(attribute, " does not exist")
+    end
+  end
+end
diff --git a/spec/models/ci/trigger_schedule_spec.rb b/spec/models/ci/trigger_schedule_spec.rb
index 6a586a4e9b1..30972f2295e 100644
--- a/spec/models/ci/trigger_schedule_spec.rb
+++ b/spec/models/ci/trigger_schedule_spec.rb
@@ -4,8 +4,6 @@ describe Ci::TriggerSchedule, models: true do
 
   it { is_expected.to belong_to(:project) }
   it { is_expected.to belong_to(:trigger) }
-  # it { is_expected.to validate_presence_of :cron }
-  # it { is_expected.to validate_presence_of :cron_time_zone }
   it { is_expected.to respond_to :ref }
 
   it 'should validate ref existence' do
@@ -26,7 +24,7 @@ describe Ci::TriggerSchedule, models: true do
     context 'when every hour' do
       let(:cron) { '0 * * * *' } # 00:00, 01:00, 02:00, ..., 23:00
 
-      it 'fails' do
+      it 'gets an error' do
         expect(trigger_schedule.errors[:cron].first).to include('can not be less than 1 hour')
       end
     end
@@ -34,7 +32,7 @@ describe Ci::TriggerSchedule, models: true do
     context 'when each six hours' do
       let(:cron) { '0 */6 * * *' } # 00:00, 06:00, 12:00, 18:00
 
-      it 'succeeds' do
+      it 'gets no errors' do
         expect(trigger_schedule.errors[:cron]).to be_empty
       end
     end
diff --git a/spec/workers/trigger_schedule_worker_spec.rb b/spec/workers/trigger_schedule_worker_spec.rb
index f0c7eeaedae..950f72a68d9 100644
--- a/spec/workers/trigger_schedule_worker_spec.rb
+++ b/spec/workers/trigger_schedule_worker_spec.rb
@@ -28,23 +28,6 @@ describe TriggerScheduleWorker do
     end
   end
 
-  context 'when there is a scheduled trigger within next_run_at and a runnign pipeline' do
-    let!(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build, :force_triggable) }
-
-    before do
-      create(:ci_pipeline, project: trigger_schedule.project, ref: trigger_schedule.ref, status: 'running')
-      worker.perform
-    end
-
-    it 'do not create a new pipeline' do
-      expect(Ci::Pipeline.count).to eq(1)
-    end
-
-    it 'do not reschedule next_run_at' do
-      expect(Ci::TriggerSchedule.last.next_run_at).to eq(trigger_schedule.next_run_at)
-    end
-  end
-
   context 'when there are no scheduled triggers within next_run_at' do
     let!(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build) }