diff --git a/app/assets/javascripts/kuroko2/suspend_schedule_handler.js b/app/assets/javascripts/kuroko2/suspend_schedule_handler.js new file mode 100644 index 00000000..7dca236e --- /dev/null +++ b/app/assets/javascripts/kuroko2/suspend_schedule_handler.js @@ -0,0 +1,9 @@ +$(function(){ + $("#new_job_suspend_schedule"). + on("ajax:success", function(e, data, status, xhr){ + $("#suspend-cron-error").hide(); + }). + on("ajax:error", function(e, xhr, status, error) { + $("#suspend-cron-error").show().html("

Error: " + xhr.responseJSON.join(' ') + "

"); + }); +}); diff --git a/app/assets/stylesheets/kuroko2/job_definitions.scss b/app/assets/stylesheets/kuroko2/job_definitions.scss index 1a43e02a..1190115a 100644 --- a/app/assets/stylesheets/kuroko2/job_definitions.scss +++ b/app/assets/stylesheets/kuroko2/job_definitions.scss @@ -23,3 +23,8 @@ color: #888; } } + +#suspend-cron-error { + display: none; + color: #843534; +} diff --git a/app/controllers/kuroko2/job_suspend_schedules_controller.rb b/app/controllers/kuroko2/job_suspend_schedules_controller.rb index 1be9df3f..7078b531 100644 --- a/app/controllers/kuroko2/job_suspend_schedules_controller.rb +++ b/app/controllers/kuroko2/job_suspend_schedules_controller.rb @@ -13,7 +13,7 @@ def create if suspend_schedule.valid? render json: suspend_schedule, status: :created else - render json: suspend_schedule, status: :bad_request + render json: suspend_schedule.errors.full_messages, status: :bad_request end end diff --git a/app/models/kuroko2/job_schedule.rb b/app/models/kuroko2/job_schedule.rb index 8e81d9ea..efcb0429 100644 --- a/app/models/kuroko2/job_schedule.rb +++ b/app/models/kuroko2/job_schedule.rb @@ -15,17 +15,16 @@ class Kuroko2::JobSchedule < Kuroko2::ApplicationRecord (?:[0-6]|(?:(?:[0-6]\-[0-6]|\*)(?:\/[0-6])?))(?:,(?:[0-6]|(?:(?:[0-6]\-[0-6]|\*)(?:\/[0-6])?)))* \z/x + CHRONO_SCHEDULE_METHODS = %i[minutes hours days months wdays] + validates :cron, format: { with: CRON_FORMAT }, uniqueness: { scope: :job_definition_id } validate :validate_cron_schedule def next(now = Time.current) - if 1.month.ago(now).future? - Kuroko2.logger.warn("Exceeds the time of criteria #{now}. (Up to 1 month since)") - return - end + return if suspended_all? next_time = Chrono::Iterator.new(self.cron, now: now).next - suspend_times = suspend_times(now, next_time) + suspend_times = suspend_times(next_time, next_time) if suspend_times.include?(next_time) self.next(next_time) @@ -51,7 +50,7 @@ def scheduled_times(time_from, time_to) end def suspend_times(time_from, time_to) - if job_definition && job_definition.job_suspend_schedules.present? + if has_suspend_schedules? job_definition.job_suspend_schedules. map { |schedule| schedule.suspend_times(time_from, time_to) }.flatten.uniq else @@ -61,6 +60,43 @@ def suspend_times(time_from, time_to) private + def suspended_all? + return false unless has_suspend_schedules? + + launch_schedule = Chrono::Schedule.new(cron) + schedule = CHRONO_SCHEDULE_METHODS.each_with_object({}) do |method, h| + h[method] = launch_schedule.public_send(method) + end + + job_definition.job_suspend_schedules.each do |suspend_schedule_model| + suspend_schedule = Chrono::Schedule.new(suspend_schedule_model.cron) + CHRONO_SCHEDULE_METHODS.each do |method| + schedule[method] -= suspend_schedule.public_send(method) + end + + # 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 suspend_schedule.wdays? && suspend_schedule.days? + case + when launch_schedule.wdays? && !launch_schedule.days? + schedule[:days] = [] + when !launch_schedule.wdays? && launch_schedule.days? + schedule[:wdays] = [] + end + end + end + + schedule.values.all?(&:empty?) + end + + def has_suspend_schedules? + job_definition && !job_definition.job_suspend_schedules.empty? + end + def validate_cron_schedule if CRON_FORMAT === cron self.next diff --git a/app/models/kuroko2/job_suspend_schedule.rb b/app/models/kuroko2/job_suspend_schedule.rb index 72fc852a..314e439a 100644 --- a/app/models/kuroko2/job_suspend_schedule.rb +++ b/app/models/kuroko2/job_suspend_schedule.rb @@ -26,9 +26,37 @@ def suspend_times(time_from, time_to) def validate_cron_schedule if Kuroko2::JobSchedule::CRON_FORMAT === cron - Chrono::Iterator.new(self.cron).next + suspend_schedule = Chrono::Schedule.new(cron) + if job_definition.job_schedules.empty? + errors.add(:cron, "needs job schedules") + else + 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 launched schedules") + end + end end - nil rescue Chrono::Fields::Base::InvalidField => e errors.add(:cron, "has invalid field: #{e.message}") end diff --git a/app/views/kuroko2/job_definitions/_list.html.slim b/app/views/kuroko2/job_definitions/_list.html.slim index 8517ffa4..7c27de5e 100644 --- a/app/views/kuroko2/job_definitions/_list.html.slim +++ b/app/views/kuroko2/job_definitions/_list.html.slim @@ -13,9 +13,12 @@ tr td= definition.id td.no-decorate= link_to definition.name, definition - - if !definition.suspended? && definition.job_schedules.present? + - if !definition.suspended? && !definition.job_schedules.empty? - next_job_schedule = definition.job_schedules.map(&:next).min - td data-value=next_job_schedule.to_i = l(next_job_schedule, format: :short) + - if next_job_schedule + td data-value=next_job_schedule.to_i = l(next_job_schedule, format: :short) + - else + td data-value="9999999999" -- - else td data-value="9999999999" -- td= labeled_status(definition.job_instances.take) diff --git a/app/views/kuroko2/job_suspend_schedules/index.html.slim b/app/views/kuroko2/job_suspend_schedules/index.html.slim index 620af1bd..7cb9b7ab 100644 --- a/app/views/kuroko2/job_suspend_schedules/index.html.slim +++ b/app/views/kuroko2/job_suspend_schedules/index.html.slim @@ -21,7 +21,10 @@ = form_for([@definition, @suspend_schedule], remote: true) do |form| .box-body #suspend-cron-field.row.form-group + .col-md-12 + div id="suspend-cron-error" .col-md-8 = form.text_field :cron, class: 'form-control script-input', placeholder: '0 9 * * *' + = javascript_include_tag 'kuroko2/suspend_schedule_handler' .col-md-4 = form.submit 'Add Suspend Schedule', class: 'btn btn-default btn-block' diff --git a/config/initializers/assets.rb b/config/initializers/assets.rb index b17190b1..9020db46 100644 --- a/config/initializers/assets.rb +++ b/config/initializers/assets.rb @@ -6,4 +6,4 @@ # Precompile additional assets. # application.js, application.css, and all non-JS/CSS in app/assets folder are already added. # Rails.application.config.assets.precompile += %w( search.js ) -Rails.application.config.assets.precompile += %w( kuroko2/instance_linker.js timeline/* network/* ) +Rails.application.config.assets.precompile += %w( kuroko2/instance_linker.js kuroko2/suspend_schedule_handler.js timeline/* network/* ) 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/features/job_definition_spec.rb b/spec/features/job_definition_spec.rb index dafcda78..69b84d8a 100644 --- a/spec/features/job_definition_spec.rb +++ b/spec/features/job_definition_spec.rb @@ -64,7 +64,21 @@ fill_in 'job_suspend_schedule_cron', with: '* * * * *' click_on 'Add Suspend Schedule' - expect(page).to have_selector('#suspend-schedules table tbody tr .log', text: '* * * * *', count: 1) + expect(page).to have_content('Cron needs job schedules') + expect(page).to have_selector('#suspend-schedules table tbody tr .log', text: '* * * * *', count: 0) + + fill_in 'job_schedule_cron', with: '* * * * *' + click_on 'Add Schedule' + expect(page).to have_selector('#schedules table tbody tr .log', text: '* * * * *', count: 1) + + fill_in 'job_suspend_schedule_cron', with: '* * * * *' + click_on 'Add Suspend Schedule' + expect(page).to have_content('Cron suspends all launched schedules') + expect(page).to have_selector('#suspend-schedules table tbody tr .log', text: '* * * * *', count: 0) + + fill_in 'job_suspend_schedule_cron', with: '* 10 * * *' + click_on 'Add Suspend Schedule' + expect(page).to have_selector('#suspend-schedules table tbody tr .log', text: '* 10 * * *', count: 1) within '#suspend-schedules' do click_on 'Delete' diff --git a/spec/models/job_schedule_spec.rb b/spec/models/job_schedule_spec.rb index 6af097bf..e6f12827 100644 --- a/spec/models/job_schedule_spec.rb +++ b/spec/models/job_schedule_spec.rb @@ -28,7 +28,7 @@ context 'With suspend_schelule' do before do - create(:job_suspend_schedule, job_definition: definition, cron: '0-29 10 * * *') + create(:job_suspend_schedule, job_definition: schedule.job_definition, cron: '0-29 10 * * *') end it 'skips suspend time range' do @@ -38,7 +38,8 @@ context 'When suspend schelules covers all schedule' do before do - create(:job_suspend_schedule, job_definition: definition, cron: '0-50 10 * * *') + suspend_schedule = create(:job_suspend_schedule, job_definition: schedule.job_definition, cron: '50 10 * * *') + suspend_schedule.update_column(:cron, '0-50 10 * * *') end it 'returns nil' do @@ -46,6 +47,54 @@ end end + context 'When schedules suspended long time ' do + let(:time) { Time.new(2016, 1, 2, 10, 0) } + let(:cron) { '0 10 1-7 * *' } + + before do + create(:job_suspend_schedule, job_definition: schedule.job_definition, cron: '* * * * 0-5') + end + + it 'returns next schedule' do + expect(schedule.next(time)).to eq(Time.new(2016, 2, 6, 10, 0)) + end + end + + context 'When suspended schedule has wdays and days' do + let(:time) { Time.new(2016, 1, 2, 10, 0) } + before do + create(:job_suspend_schedule, job_definition: schedule.job_definition, cron: '* * 1 * 0') # suspend every sunday or first day of month + end + + context 'If the schedule has days only' do + let(:cron) { '0 10 1 * *' } + it 'returns nil' do + expect(schedule.next(time)).to be_nil + end + end + + context 'If the schedule has wdays only' do + let(:cron) { '0 10 * * 0' } + it 'returns nil' do + expect(schedule.next(time)).to be_nil + end + end + + context 'If the schedule has wdays and days' do + let(:cron) { '0 10 2 * 0' } + it 'returns next schedule' do + expect(schedule.next(time)).to eq(Time.new(2016, 2, 2, 10, 0)) + end + end + + context 'If the schedule has wdays and days' do + let(:cron) { '0 10 1 * 1' } + it 'returns next schedule' do + expect(schedule.next(time)).to eq(Time.new(2016, 1, 4, 10, 0)) + end + end + end + context 'With invalid date' do let(:cron) { '* * 31 2 *' } let(:time) { Time.new(2016, 2, 28, 10, 0) } 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) }