From 6d42b724b09e207e9fd0d6e36002ca573d6493f5 Mon Sep 17 00:00:00 2001 From: Eisuke Oishi Date: Wed, 5 Jul 2017 12:33:39 +0900 Subject: [PATCH] Add all schedules suspended validation to JobSuspendSchedule --- app/models/kuroko2/job_suspend_schedule.rb | 33 +++++++++++++- .../job_suspend_schedules_controller_spec.rb | 18 ++++---- spec/factories/job_schedule_factory.rb | 2 + .../factories/job_suspend_schedule_factory.rb | 5 +-- spec/models/job_suspend_schedule_spec.rb | 44 ++++++++++++++++--- 5 files changed, 81 insertions(+), 21 deletions(-) diff --git a/app/models/kuroko2/job_suspend_schedule.rb b/app/models/kuroko2/job_suspend_schedule.rb index 72fc852a..c3cee190 100644 --- a/app/models/kuroko2/job_suspend_schedule.rb +++ b/app/models/kuroko2/job_suspend_schedule.rb @@ -26,9 +26,38 @@ def suspend_times(time_from, time_to) def validate_cron_schedule if Kuroko2::JobSchedule::CRON_FORMAT === cron - Chrono::Iterator.new(self.cron).next + unless job_definition.job_schedules.empty? + suspend_schedule = Chrono::Schedule.new(cron) + + schedule = job_definition.job_schedules.each_with_object({}) do |launch_schedule_model, h| + launch_schedule = Chrono::Schedule.new(launch_schedule_model.cron) + Kuroko2::JobSchedule::CHRONO_SCHEDULE_METHODS.each do |method| + h[method] ||= [] + + suspend_schedule_list = suspend_schedule.public_send(method) + # https://linux.die.net/man/5/crontab + # > Note: The day of a command's execution can be specified by two fields + # > day of month, and day of week. If both fields are restricted (ie, aren't *), + # > the command will be run when either field matches the current time. + # > For example, "30 4 1,15 * 5" would cause a command to be run at 4:30 am + # > on the 1st and 15th of each month, plus every Friday. + if launch_schedule.wdays? && launch_schedule.days? + if (method == :wdays && suspend_schedule.wdays? && !suspend_schedule.days?) || (method == :days && !suspend_schedule.wdays? && suspend_schedule.days?) + suspend_schedule_list = [] + end + end + + h[method] |= launch_schedule.public_send(method) - suspend_schedule_list + end + end + + if schedule.values.all?(&:empty?) + errors.add(:cron, "suspends all schedules") + end + end + else + errors.add(:cron, "has invalid format") end - nil rescue Chrono::Fields::Base::InvalidField => e errors.add(:cron, "has invalid field: #{e.message}") end diff --git a/spec/controllers/job_suspend_schedules_controller_spec.rb b/spec/controllers/job_suspend_schedules_controller_spec.rb index 093a2ef0..ecd5fb4b 100644 --- a/spec/controllers/job_suspend_schedules_controller_spec.rb +++ b/spec/controllers/job_suspend_schedules_controller_spec.rb @@ -4,24 +4,22 @@ routes { Kuroko2::Engine.routes } before { sign_in } - - let(:schedules) { create_list(:job_suspend_schedule, 1) } - let(:definition) { create(:job_definition, job_suspend_schedules: schedules) } + let(:schedule) { create(:job_schedule, cron: '0 10 * * *') } + let(:suspend_schedule) { create(:job_suspend_schedule, job_definition: schedule.job_definition, cron: '0 10 * * 1') } describe '#index' do it do - get :index, params: { job_definition_id: definition.id } + get :index, params: { job_definition_id: suspend_schedule.job_definition.id } expect(response).to have_http_status(:ok) expect(assigns(:suspend_schedule)).to be_new_record - expect(assigns(:suspend_schedules)).to eq schedules + expect(assigns(:suspend_schedules)).to eq [suspend_schedule] end end describe '#create' do it do - post :create, params: { job_definition_id: definition.id, job_suspend_schedule: { cron: '* * * * *' } } - + post :create, params: { job_definition_id: suspend_schedule.job_definition.id, job_suspend_schedule: { cron: '0 10 * * 0' } } expect(response).to have_http_status(:created) end end @@ -29,11 +27,11 @@ describe '#destroy' do it do - delete :destroy, params: { job_definition_id: definition.id, id: schedules.first.id } - definition.reload + delete :destroy, params: { job_definition_id: suspend_schedule.job_definition.id, id: suspend_schedule.id } + schedule.job_definition.reload expect(response).to have_http_status(:ok) - expect(definition.job_schedules.size).to be 0 + expect(schedule.job_definition.job_suspend_schedules.size).to be 0 end end end diff --git a/spec/factories/job_schedule_factory.rb b/spec/factories/job_schedule_factory.rb index 1a2e882e..d3e7e91a 100644 --- a/spec/factories/job_schedule_factory.rb +++ b/spec/factories/job_schedule_factory.rb @@ -6,5 +6,7 @@ "#{n % 60} #{hour} #{day} * *" end + + job_definition end end diff --git a/spec/factories/job_suspend_schedule_factory.rb b/spec/factories/job_suspend_schedule_factory.rb index 49579b98..807b502e 100644 --- a/spec/factories/job_suspend_schedule_factory.rb +++ b/spec/factories/job_suspend_schedule_factory.rb @@ -1,8 +1,5 @@ FactoryGirl.define do factory :job_suspend_schedule, class: Kuroko2::JobSuspendSchedule do - sequence(:cron) do |n| - hour = n / 60 >= 1 ? (n / 60) : '*' - "#{n % 60} #{hour} * * *" - end + job_definition end end diff --git a/spec/models/job_suspend_schedule_spec.rb b/spec/models/job_suspend_schedule_spec.rb index ac69c93e..02b0365c 100644 --- a/spec/models/job_suspend_schedule_spec.rb +++ b/spec/models/job_suspend_schedule_spec.rb @@ -1,20 +1,54 @@ require 'rails_helper' describe Kuroko2::JobSuspendSchedule do + let(:launch_schedule) { create(:job_schedule, cron: '* * * * *') } + describe '#valid?' do it 'accepts only CRON notation' do - expect(Kuroko2::JobSuspendSchedule.new(cron: '* * * * *')).to be_valid - expect(Kuroko2::JobSuspendSchedule.new(cron: '1,2-3,*,*/4,5-6/7 1,2-3,*,*/4,5-6/7 1,2-3,*,*/4,5-6/7 1,2-3,*,*/4,5-6/7 1,2-3,*,*/4,1-3/5')).to be_valid - expect(Kuroko2::JobSuspendSchedule.new(cron: '* * * *')).not_to be_valid + expect(Kuroko2::JobSuspendSchedule.new(cron: '0-30 10 * * *', job_definition: launch_schedule.job_definition)).to be_valid + expect(Kuroko2::JobSuspendSchedule.new(cron: '1,2-3,*/4,5-6/7 1,2-3,*,*/4,5-6/7 1,2-3,*,*/4,5-6/7 1,2-3,*,*/4,5-6/7 1,2-3,*,*/4,1-3/5', job_definition: launch_schedule.job_definition)).to be_valid + expect(Kuroko2::JobSuspendSchedule.new(cron: '* * * *', job_definition: launch_schedule.job_definition)).not_to be_valid end it 'accepts only valid CRON notation' do - expect(Kuroko2::JobSuspendSchedule.new(cron: '5-0 * * * *')).not_to be_valid + expect(Kuroko2::JobSuspendSchedule.new(cron: '5-0 * * * *', job_definition: launch_schedule.job_definition)).not_to be_valid + end + + context 'when suspend all schedules' do + let(:launch_schedule) { create(:job_schedule, cron: '5 10 * * 0') } + + it 'does not accept' do + expect(Kuroko2::JobSuspendSchedule.new(cron: '* * * * 0', job_definition: launch_schedule.job_definition)).not_to be_valid + end + + context 'when launch schedule has wdays and days' do + let(:launch_schedule) { create(:job_schedule, cron: '5 10 1 * 1') } + + it 'does not accept' do + expect(Kuroko2::JobSuspendSchedule.new(cron: '* * 1 * 1', job_definition: launch_schedule.job_definition)).not_to be_valid + end + + context 'when supend_schedule has only wdays' do + let(:launch_schedule) { create(:job_schedule, cron: '5 10 1 * 1') } + + it 'accepts' do + expect(Kuroko2::JobSuspendSchedule.new(cron: '* * * * 1', job_definition: launch_schedule.job_definition)).to be_valid + end + end + + context 'when supend_schedule has only days' do + let(:launch_schedule) { create(:job_schedule, cron: '5 10 1 * 1') } + + it 'accepts' do + expect(Kuroko2::JobSuspendSchedule.new(cron: '* * 1 * *', job_definition: launch_schedule.job_definition)).to be_valid + end + end + end end end describe '#suspend_times' do - let(:suspend_schedule) { create(:job_suspend_schedule, cron: '* 10 * * *') } + let(:suspend_schedule) { create(:job_suspend_schedule, cron: '* 10 * * *', job_definition: launch_schedule.job_definition) } let(:time_from) { Time.new(2016, 1, 1, 10, 0, 0) } let(:time_to) { Time.new(2016, 1, 1, 10, 5, 0) }