From 778f8d2cd872227b3a4b04936d42684abc0eb417 Mon Sep 17 00:00:00 2001 From: Susan Wright Date: Mon, 23 Oct 2017 18:25:41 -0600 Subject: [PATCH 1/6] Do not allow multiple events w/ the same name --- lib/clockwork/manager.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/clockwork/manager.rb b/lib/clockwork/manager.rb index 55ec497..a35beb0 100644 --- a/lib/clockwork/manager.rb +++ b/lib/clockwork/manager.rb @@ -1,11 +1,13 @@ module Clockwork class Manager class NoHandlerDefined < RuntimeError; end + class DuplicateEventNames < RunTimeError; end attr_reader :config def initialize @events = [] + @event_names = [] @callbacks = {} @config = default_configuration @handler = nil @@ -163,8 +165,12 @@ def events_to_run(t) @events.select{ |event| event.run_now?(t) } end - def register(period, job, block, options) + def register(period, job, block, options, skip_duplicate_check = false) event = Event.new(self, period, job, block || handler, options) + if @event_names.include?(job) + raise DuplicateNameError unless skip_duplicate_check + end + @event_names << job @events << event event end @@ -173,7 +179,7 @@ def every_with_multiple_times(period, job, options={}, &block) each_options = options.clone options[:at].each do |at| each_options[:at] = at - register(period, job, block, each_options) + register(period, job, block, each_options, true) end end end From 2be2e8b48fbbfddb8ddbab0335516992d5af2673 Mon Sep 17 00:00:00 2001 From: Susan Wright Date: Mon, 23 Oct 2017 18:30:01 -0600 Subject: [PATCH 2/6] Prevent collisions among unnamed jobs --- lib/clockwork/manager.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/clockwork/manager.rb b/lib/clockwork/manager.rb index a35beb0..39d4929 100644 --- a/lib/clockwork/manager.rb +++ b/lib/clockwork/manager.rb @@ -8,6 +8,7 @@ class DuplicateEventNames < RunTimeError; end def initialize @events = [] @event_names = [] + @incrementer = 0 @callbacks = {} @config = default_configuration @handler = nil @@ -52,6 +53,12 @@ def every(period, job='unnamed', options={}, &block) options = job job = "unnamed" end + + if job == "unnamed" + job += @incrementer + @incrementer += 1 + end + if options[:at].respond_to?(:each) every_with_multiple_times(period, job, options, &block) else From b0a4b32cfa888abd6eb02cbdf48ce43064ab0a8b Mon Sep 17 00:00:00 2001 From: Susan Wright Date: Mon, 23 Oct 2017 18:35:38 -0600 Subject: [PATCH 3/6] Use proper Error type, underscore unnamed jobs --- lib/clockwork/manager.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/clockwork/manager.rb b/lib/clockwork/manager.rb index 39d4929..30cb640 100644 --- a/lib/clockwork/manager.rb +++ b/lib/clockwork/manager.rb @@ -1,7 +1,7 @@ module Clockwork class Manager class NoHandlerDefined < RuntimeError; end - class DuplicateEventNames < RunTimeError; end + class DuplicateEventNames < RuntimeError; end attr_reader :config @@ -55,7 +55,7 @@ def every(period, job='unnamed', options={}, &block) end if job == "unnamed" - job += @incrementer + job += "_#{@incrementer}" @incrementer += 1 end From 014de7a0fd9c79b7a982b6dfdc3b0c6b3027fff2 Mon Sep 17 00:00:00 2001 From: Susan Wright Date: Mon, 23 Oct 2017 18:39:26 -0600 Subject: [PATCH 4/6] Add specs around unnamed jobs --- test/manager_test.rb | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/test/manager_test.rb b/test/manager_test.rb index c102c43..17bd358 100644 --- a/test/manager_test.rb +++ b/test/manager_test.rb @@ -155,12 +155,28 @@ def assert_wont_run(t) it "should accept unnamed job" do event = @manager.every(1.minute) - assert_equal 'unnamed', event.job + assert_equal 'unnamed_0', event.job end it "should accept options without job name" do event = @manager.every(1.minute, {}) - assert_equal 'unnamed', event.job + assert_equal 'unnamed_0', event.job + end + + it "should accept multiple unnamed jobs" do + event = @manager.every(1.minute) + event2 = @manager.every(2.minutes) + + assert_equal "unnamed_0", event.job + assert_equal "unnamed_1", event2.job + end + + it "should accept multiple options without job name" do + event = @manager.every(1.minute, {}) + event2 = @manager.every(2.minute, {}) + + assert_equal "unnamed_0", event.job + assert_equal "unnamed_1", event2.job end describe ':at option' do From 003a1189f4aa83853840872a313c46952becdc9b Mon Sep 17 00:00:00 2001 From: Susan Wright Date: Mon, 23 Oct 2017 18:42:21 -0600 Subject: [PATCH 5/6] Tests around enforcement of duplicate names --- lib/clockwork/manager.rb | 2 +- test/manager_test.rb | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/clockwork/manager.rb b/lib/clockwork/manager.rb index 30cb640..65584a3 100644 --- a/lib/clockwork/manager.rb +++ b/lib/clockwork/manager.rb @@ -175,7 +175,7 @@ def events_to_run(t) def register(period, job, block, options, skip_duplicate_check = false) event = Event.new(self, period, job, block || handler, options) if @event_names.include?(job) - raise DuplicateNameError unless skip_duplicate_check + raise(DuplicateEventNames, "#{job} is identical to another event") unless skip_duplicate_check end @event_names << job @events << event diff --git a/test/manager_test.rb b/test/manager_test.rb index 17bd358..7eef6ac 100644 --- a/test/manager_test.rb +++ b/test/manager_test.rb @@ -80,6 +80,13 @@ def assert_wont_run(t) end end + it "aborts when there are multiple jobs with the same name" do + assert_raises(Clockwork::Manager::DuplicateEventNames) do + @manager.every(1.minute, "jobx") + @manager.every(2.minutes, "jobx") + end + end + it "general handler" do $set_me = 0 @manager.handler { $set_me = 1 } From 2451e906b2472f2ba655daf733f1510629e09c50 Mon Sep 17 00:00:00 2001 From: Susan Wright Date: Mon, 23 Oct 2017 19:29:08 -0600 Subject: [PATCH 6/6] Leave Database structures alone --- lib/clockwork/database_events/manager.rb | 2 +- test/database_events/synchronizer_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/clockwork/database_events/manager.rb b/lib/clockwork/database_events/manager.rb index 3231d6f..6141255 100644 --- a/lib/clockwork/database_events/manager.rb +++ b/lib/clockwork/database_events/manager.rb @@ -8,7 +8,7 @@ def unregister(event) @events.delete(event) end - def register(period, job, block, options) + def register(period, job, block, options, _skip_duplicate_check = false) @events << if options[:from_database] synchronizer = options.fetch(:synchronizer) model_attributes = options.fetch(:model_attributes) diff --git a/test/database_events/synchronizer_test.rb b/test/database_events/synchronizer_test.rb index 210c4d1..3f9ba80 100644 --- a/test/database_events/synchronizer_test.rb +++ b/test/database_events/synchronizer_test.rb @@ -164,7 +164,7 @@ def log(msg); end # silence log output end describe "when #name is defined" do - it 'runs daily event with at from databse only once' do + it 'runs daily event with at from database only once' do DatabaseEventModel.create(:frequency => 1.day, :at => next_minute(@now).strftime('%H:%M')) setup_sync(model: DatabaseEventModel, :every => @sync_frequency, :events_run => @events_run)