From 5b7f2bb7044817bbc00ac234946cc0c31d2679cd Mon Sep 17 00:00:00 2001 From: Cristian Dotta Date: Thu, 7 Jul 2016 13:40:39 -0300 Subject: [PATCH] Add next time execution at recurring jobs view --- lib/sidekiq-scheduler/job_presenter.rb | 54 +++++++++++++++ lib/sidekiq-scheduler/web.rb | 4 +- lib/sidekiq/scheduler.rb | 21 +++++- spec/sidekiq-scheduler/web_spec.rb | 44 ++++++++----- spec/sidekiq/scheduler_spec.rb | 91 +++++++++++++++++++++----- web/locales/cs.yml | 3 +- web/locales/en.yml | 3 +- web/locales/es.yml | 3 +- web/views/recurring_jobs.erb | 18 ++--- 9 files changed, 195 insertions(+), 46 deletions(-) create mode 100644 lib/sidekiq-scheduler/job_presenter.rb diff --git a/lib/sidekiq-scheduler/job_presenter.rb b/lib/sidekiq-scheduler/job_presenter.rb new file mode 100644 index 00000000..1f31ac99 --- /dev/null +++ b/lib/sidekiq-scheduler/job_presenter.rb @@ -0,0 +1,54 @@ +require 'sidekiq/web_helpers' + +module SidekiqScheduler + class JobPresenter + attr_reader :name + + include Sidekiq::WebHelpers + + def initialize(name, attributes) + @name = name + @attributes = attributes + end + + # Returns the next time execution for the job + # + # @return [String] with the job's next time + def next_time + execution_time = Sidekiq.redis { |r| r.hget(Sidekiq::Scheduler.next_times_key, name) } + + relative_time(Time.parse(execution_time)) if execution_time + end + + # Returns the interval for the job + # + # @return [String] with the job's interval + def interval + @attributes.fetch('cron', @attributes['every']) + end + + # Returns the queue of the job + # + # @return [String] with the job's queue + def queue + @attributes.fetch('queue', 'default') + end + + # Delegates the :[] method to the attributes' hash + # + # @return [String] with the value for that key + def [](key) + @attributes[key] + end + + # Builds the presenter instances for the schedule hash + # + # @param schedule_hash [Hash] with the redis schedule + # @return [Array] an array with the instances of presenters + def self.build_collection(schedule_hash) + Hash(schedule_hash).map do |name, job_spec| + new(name, job_spec) + end + end + end +end diff --git a/lib/sidekiq-scheduler/web.rb b/lib/sidekiq-scheduler/web.rb index 832f8503..a2e0a061 100644 --- a/lib/sidekiq-scheduler/web.rb +++ b/lib/sidekiq-scheduler/web.rb @@ -1,3 +1,5 @@ +require_relative 'job_presenter' + module SidekiqScheduler # Hook into *Sidekiq::Web* Sinatra app which adds a new '/recurring-jobs' page @@ -6,7 +8,7 @@ module Web def self.registered(app) app.get '/recurring-jobs' do - @schedule = (Sidekiq.schedule! || []) + @presented_jobs = JobPresenter.build_collection(Sidekiq.schedule!) erb File.read(File.join(VIEW_PATH, 'recurring_jobs.erb')) end diff --git a/lib/sidekiq/scheduler.rb b/lib/sidekiq/scheduler.rb index d0c706f0..dfbe1054 100644 --- a/lib/sidekiq/scheduler.rb +++ b/lib/sidekiq/scheduler.rb @@ -113,12 +113,17 @@ def self.load_schedule_job(name, config) # We want rufus_scheduler to return a job object, not a job id opts = { :job => true } - @@scheduled_jobs[name] = self.rufus_scheduler.send(interval_type, *args, opts) do |job, time| + rufus_job = self.rufus_scheduler.send(interval_type, *args, opts) do |job, time| config.delete(interval_type) idempotent_job_enqueue(name, time, config) + update_job_next_time(name, job.next_time) end + @@scheduled_jobs[name] = rufus_job + + update_job_next_time(name, rufus_job.next_time) + interval_defined = true break @@ -150,6 +155,14 @@ def self.idempotent_job_enqueue(job_name, time, config) end end + # Pushes job's next time execution + # + # @param [String] name The job's name + # @param [Time] next_time The job's next time execution + def self.update_job_next_time(name, next_time) + Sidekiq.redis { |r| r.hset(next_times_key, name, next_time) } + end + # Returns true if the given schedule config hash matches the current # ENV['RAILS_ENV'] def self.rails_env_matches?(config) @@ -320,5 +333,11 @@ def self.remove_elder_job_instances(job_name) def self.pushed_job_key(job_name) "sidekiq-scheduler:pushed:#{job_name}" end + + # Returns the key of the Redis hash for job's execution times hash + # + def self.next_times_key + 'sidekiq-scheduler:next_times' + end end end diff --git a/spec/sidekiq-scheduler/web_spec.rb b/spec/sidekiq-scheduler/web_spec.rb index e84d67c0..4e6e2210 100644 --- a/spec/sidekiq-scheduler/web_spec.rb +++ b/spec/sidekiq-scheduler/web_spec.rb @@ -3,6 +3,7 @@ describe Sidekiq::Web do include Rack::Test::Methods + before { Sidekiq.redis(&:flushall) } def app Sidekiq::Web @@ -30,27 +31,40 @@ def app Sidekiq.schedule = jobs end - it 'shows schedule' do - get '/recurring-jobs' + describe '/recurring-jobs' do + it 'shows schedule' do + get '/recurring-jobs' - expect(last_response).to be_ok + expect(last_response).to be_ok - expect(last_response.body).to match(/Foo Job/) - expect(last_response.body).to match(/FooClass/) - expect(last_response.body).to match(/0 \* \* \* \* US\/Eastern/) - expect(last_response.body).to match(/default/) - expect(last_response.body).to match(/\[42\]/) - expect(last_response.body).to match(/Does foo things\./) + expect(last_response.body).to match(/Foo Job/) + expect(last_response.body).to match(/FooClass/) + expect(last_response.body).to match(/0 \* \* \* \* US\/Eastern/) + expect(last_response.body).to match(/default/) + expect(last_response.body).to match(/\[42\]/) + expect(last_response.body).to match(/Does foo things\./) - expect(last_response.body).to match(/Bar Job/) - expect(last_response.body).to match(/BarClass/) - expect(last_response.body).to match(/1h/) - expect(last_response.body).to match(/special/) - expect(last_response.body).to match(/\[\"foo\", \"bar\"\]/) + expect(last_response.body).to match(/Bar Job/) + expect(last_response.body).to match(/BarClass/) + expect(last_response.body).to match(/1h/) + expect(last_response.body).to match(/special/) + expect(last_response.body).to match(/\[\"foo\", \"bar\"\]/) - expect(last_response.body).to match(/Enqueue now/) + expect(last_response.body).to match(/Enqueue now/) + end + + context 'when the next execution time is setted' do + before { Sidekiq::Scheduler.update_job_next_time('Foo Job', "2016-07-11T13:29:47Z") } + + it 'shows the next time for the job' do + get '/recurring-jobs' + + expect(last_response.body).to match(/2016-07-11T13:29:47Z/) + end + end end + it 'enqueues particular job' do job_name = jobs.keys.first job = jobs[job_name] diff --git a/spec/sidekiq/scheduler_spec.rb b/spec/sidekiq/scheduler_spec.rb index 508aeb6a..7620750e 100644 --- a/spec/sidekiq/scheduler_spec.rb +++ b/spec/sidekiq/scheduler_spec.rb @@ -277,85 +277,140 @@ end describe '.load_schedule_job' do + + let(:job_name) { 'some_job' } + let(:next_time_execution) do + Sidekiq.redis { |r| r.hexists(Sidekiq::Scheduler.next_times_key, job_name) } + end + context 'cron schedule' do it 'loads correctly with no options' do - Sidekiq::Scheduler.load_schedule_job('some_job', ScheduleFaker.cron_schedule) + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.cron_schedule) expect(Sidekiq::Scheduler.rufus_scheduler.jobs.size).to be(1) - expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq(%w(some_job)) + expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq([job_name]) + end + + it 'stores the next time execution correctly with no options' do + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.cron_schedule) + expect(next_time_execution).to be end it 'loads correctly with options' do - Sidekiq::Scheduler.load_schedule_job('other_job', + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.cron_schedule('allow_overlapping' => 'true')) expect(Sidekiq::Scheduler.rufus_scheduler.jobs.size).to be(1) - expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq(%w(other_job)) - expect(Sidekiq::Scheduler.scheduled_jobs['other_job'].params.keys). + expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq([job_name]) + expect(Sidekiq::Scheduler.scheduled_jobs[job_name].params.keys). to include(:allow_overlapping) end + it 'stores the next time execution correctly with options' do + Sidekiq::Scheduler.load_schedule_job(job_name, + ScheduleFaker.cron_schedule('allow_overlapping' => 'true')) + expect(next_time_execution).to be + end + it 'does not load the schedule with an empty cron' do - Sidekiq::Scheduler.load_schedule_job('empty_cron_job', + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.cron_schedule('cron' => '')) expect(Sidekiq::Scheduler.rufus_scheduler.jobs.size).to be(0) expect(Sidekiq::Scheduler.scheduled_jobs.keys).to be_empty end + + it 'does not store the next time execution correctly with options' do + Sidekiq::Scheduler.load_schedule_job(job_name, + ScheduleFaker.cron_schedule('cron' => '')) + + expect(next_time_execution).not_to be + end end context 'every schedule' do it 'loads correctly with no options' do - Sidekiq::Scheduler.load_schedule_job('some_job', ScheduleFaker.every_schedule) + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.every_schedule) expect(Sidekiq::Scheduler.rufus_scheduler.jobs.size).to be(1) - expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq(%w(some_job)) + expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq([job_name]) + end + + it 'stores the next time execution correctly with no options' do + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.every_schedule) + expect(next_time_execution).to be end it 'loads correctly with options' do - Sidekiq::Scheduler.load_schedule_job('some_job', + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.every_schedule({'first_in' => '60s'})) expect(Sidekiq::Scheduler.rufus_scheduler.jobs.size).to be(1) - expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq(%w(some_job)) - expect(Sidekiq::Scheduler.scheduled_jobs['some_job'].params.keys). + expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq([job_name]) + expect(Sidekiq::Scheduler.scheduled_jobs[job_name].params.keys). to include(:first_in) end + + it 'stores the next time execution correctly with options' do + Sidekiq::Scheduler.load_schedule_job(job_name, + ScheduleFaker.every_schedule({'first_in' => '60s'})) + expect(next_time_execution).to be + end end context 'at schedule' do it 'loads correctly' do - Sidekiq::Scheduler.load_schedule_job('some_job', ScheduleFaker.at_schedule) + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.at_schedule) expect(Sidekiq::Scheduler.rufus_scheduler.jobs.size).to be(1) - expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq(%w(some_job)) + expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq([job_name]) + end + + it 'stores the next time execution correctly' do + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.at_schedule) + expect(next_time_execution).to be end end context 'in schedule' do it 'load_schedule_job with in' do - Sidekiq::Scheduler.load_schedule_job('some_job', ScheduleFaker.in_schedule) + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.in_schedule) expect(Sidekiq::Scheduler.rufus_scheduler.jobs.size).to be(1) - expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq(%w(some_job)) + expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq([job_name]) + end + + it 'stores the next time execution correctly' do + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.in_schedule) + expect(next_time_execution).to be end end context 'interval schedule' do it 'loads correctly' do - Sidekiq::Scheduler.load_schedule_job('some_job', ScheduleFaker.interval_schedule) + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.interval_schedule) expect(Sidekiq::Scheduler.rufus_scheduler.jobs.size).to be(1) - expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq(%w(some_job)) + expect(Sidekiq::Scheduler.scheduled_jobs.keys).to eq([job_name]) + end + + it 'stores the next time execution correctly' do + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.interval_schedule) + expect(next_time_execution).to be end end it 'does not load without a timing option' do - Sidekiq::Scheduler.load_schedule_job('some_job', ScheduleFaker.invalid_schedule) + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.invalid_schedule) expect(Sidekiq::Scheduler.rufus_scheduler.jobs.size).to be(0) expect(Sidekiq::Scheduler.scheduled_jobs.keys).to be_empty end + + it 'does not stores the next time execution without a timing option' do + Sidekiq::Scheduler.load_schedule_job(job_name, ScheduleFaker.invalid_schedule) + expect(next_time_execution).not_to be + end end describe '.idempotent_job_enqueue' do diff --git a/web/locales/cs.yml b/web/locales/cs.yml index de18933d..fa630e35 100644 --- a/web/locales/cs.yml +++ b/web/locales/cs.yml @@ -6,4 +6,5 @@ cs: class: Třída queue: Fronta arguments: Argumenty - enqueue_now: Zařadit nyní \ No newline at end of file + enqueue_now: Zařadit nyní + next_time: Příště diff --git a/web/locales/en.yml b/web/locales/en.yml index 7ab047e1..867fa992 100644 --- a/web/locales/en.yml +++ b/web/locales/en.yml @@ -6,4 +6,5 @@ en: class: Class queue: Queue arguments: Arguments - enqueue_now: Enqueue now \ No newline at end of file + enqueue_now: Enqueue now + next_time: Next Time diff --git a/web/locales/es.yml b/web/locales/es.yml index a95b6fd9..63386a32 100644 --- a/web/locales/es.yml +++ b/web/locales/es.yml @@ -6,4 +6,5 @@ es: class: Clase queue: Cola arguments: Argumentos - enqueue_now: Encolar ahora \ No newline at end of file + enqueue_now: Encolar ahora + next_time: Próxima ejecución diff --git a/web/views/recurring_jobs.erb b/web/views/recurring_jobs.erb index 341b1244..112a1dab 100644 --- a/web/views/recurring_jobs.erb +++ b/web/views/recurring_jobs.erb @@ -10,23 +10,25 @@ <%= t('class') %> <%= t('queue') %> <%= t('arguments') %> + <%= t('next_time') %> - <% @schedule.each do |name, job_spec| %> + <% @presented_jobs.each do |job| %> - <%= name %> - <%= job_spec['description'] %> - <%= job_spec.fetch 'cron', job_spec['every'] %> - <%= job_spec['class'] %> + <%= job.name %> + <%= job['description'] %> + <%= job.interval %> + <%= job['class'] %> - <%= job_spec.fetch('queue', 'default') %> + <%= job.queue %> - <%= job_spec['args'] %> + <%= job['args'] %> + <%= job.next_time %> - + <%= t('enqueue_now') %>