diff --git a/app/models/ci/trigger_schedule.rb b/app/models/ci/trigger_schedule.rb index 7efaa228a93..1b3b73971bb 100644 --- a/app/models/ci/trigger_schedule.rb +++ b/app/models/ci/trigger_schedule.rb @@ -7,15 +7,42 @@ module Ci belongs_to :project belongs_to :trigger + validates :trigger, presence: true + validates :cron, presence: true + validates :cron_time_zone, presence: true + validate :check_cron + validate :check_ref + + after_create :schedule_next_run! + def schedule_next_run! next_time = Ci::CronParser.new(cron, cron_time_zone).next_time_from_now if next_time.present? - update_attributes(next_run_at: next_time) + update!(next_run_at: next_time) end end - # def update_last_run! - # update_attributes(last_run_at: Time.now) - # end + private + + def check_cron + cron_parser = Ci::CronParser.new(cron, cron_time_zone) + is_valid_cron, is_valid_cron_time_zone = cron_parser.validation + + 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 (cron_parser.next_time_from_now - Time.now).abs < 1.hour + self.errors.add(:cron, " can not be less than 1 hour") + end + end + + def check_ref + if !trigger.ref.present? + self.errors.add(:ref, " is empty") + elsif trigger.project.repository.ref_exists?(trigger.ref) + self.errors.add(:ref, " does not exist") + end + end end end diff --git a/lib/ci/cron_parser.rb b/lib/ci/cron_parser.rb index 24c4dd676dc..b0ffb48dabc 100644 --- a/lib/ci/cron_parser.rb +++ b/lib/ci/cron_parser.rb @@ -6,20 +6,22 @@ module Ci end def next_time_from_now - cronLine = try_parse_cron + cronLine = try_parse_cron(@cron, @cron_time_zone) return nil unless cronLine.present? cronLine.next_time end - def valid_syntax? - try_parse_cron.present? ? true : false + def validation + is_valid_cron = try_parse_cron(@cron, 'Europe/Istanbul').present? + is_valid_cron_time_zone = try_parse_cron('* * * * *', @cron_time_zone).present? + return is_valid_cron, is_valid_cron_time_zone end private - def try_parse_cron + def try_parse_cron(cron, cron_time_zone) begin - Rufus::Scheduler.parse("#{@cron} #{@cron_time_zone}") + Rufus::Scheduler.parse("#{cron} #{cron_time_zone}") rescue nil end diff --git a/spec/factories/ci/trigger_schedules.rb b/spec/factories/ci/trigger_schedules.rb index f909e343bf2..0dd397435c1 100644 --- a/spec/factories/ci/trigger_schedules.rb +++ b/spec/factories/ci/trigger_schedules.rb @@ -4,18 +4,20 @@ FactoryGirl.define do trigger factory: :ci_trigger trait :force_triggable do - next_run_at Time.now - 1.month + after(:create) do |trigger_schedule, evaluator| + trigger_schedule.next_run_at -= 1.month + end end trait :cron_nightly_build do cron '0 1 * * *' cron_time_zone 'Europe/Istanbul' - next_run_at do # TODO: Use CronParser - time = Time.now.in_time_zone(cron_time_zone) - time = time + 1.day if time.hour > 1 - time = time.change(sec: 0, min: 0, hour: 1) - time - end + # next_run_at do # TODO: Use CronParser + # time = Time.now.in_time_zone(cron_time_zone) + # time = time + 1.day if time.hour > 1 + # time = time.change(sec: 0, min: 0, hour: 1) + # time + # end end trait :cron_weekly_build do diff --git a/spec/factories/ci/triggers.rb b/spec/factories/ci/triggers.rb index 1feaa9b9fa1..d38800b58f7 100644 --- a/spec/factories/ci/triggers.rb +++ b/spec/factories/ci/triggers.rb @@ -1,7 +1,7 @@ FactoryGirl.define do factory :ci_trigger_without_token, class: Ci::Trigger do factory :ci_trigger do - token { SecureRandom.hex(10) } + token { SecureRandom.hex(15) } end end end diff --git a/spec/models/ci/trigger_schedule_spec.rb b/spec/models/ci/trigger_schedule_spec.rb index 14b8530a65b..d8b6bd93954 100644 --- a/spec/models/ci/trigger_schedule_spec.rb +++ b/spec/models/ci/trigger_schedule_spec.rb @@ -1,29 +1,41 @@ require 'spec_helper' describe Ci::TriggerSchedule, models: true do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:trigger) { create(:ci_trigger, owner: user, project: project, ref: 'master') } describe 'associations' do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:trigger) } end - describe '#schedule_next_run!' do - subject { trigger_schedule.schedule_next_run! } + describe 'validation' do + let(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build, trigger: trigger) } - let(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build, next_run_at: nil) } + it { expect(trigger_schedule).to validate_presence_of(:trigger) } + it { is_expected.to validate_presence_of(:cron) } + it { is_expected.to validate_presence_of(:cron_time_zone) } - it 'updates next_run_at' do - is_expected.not_to be_nil + it '#check_cron' do + subject.cron = 'Hack' + subject.valid? + subject.errors[:screen_name].to include(' is invalid syntax') + end + + it '#check_ref' do end end - # describe '#update_last_run!' do - # subject { scheduled_trigger.update_last_run! } + describe '#schedule_next_run!' do + let(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build, next_run_at: nil, trigger: trigger) } - # let(:scheduled_trigger) { create(:ci_scheduled_trigger, :cron_nightly_build, last_run_at: nil) } + before do + trigger_schedule.schedule_next_run! + end - # it 'updates last_run_at' do - # is_expected.not_to be_nil - # end - # end + it 'updates next_run_at' do + expect(Ci::TriggerSchedule.last.next_run_at).not_to be_nil + end + end end