From f52ac8d67b5630c5614762394a55dce32300af66 Mon Sep 17 00:00:00 2001 From: Jingta Date: Fri, 5 May 2017 18:50:33 -0700 Subject: [PATCH 1/6] Updated the job class to use parent values if set Refactor attributes to fetch from self, the parent class, the config, in that order --- lib/easy_stalk/job.rb | 24 ++++++++++++++++------ spec/lib/easy_stalk/job_spec.rb | 36 ++++++++++++++++++++++++++++----- 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/lib/easy_stalk/job.rb b/lib/easy_stalk/job.rb index 3cab108..383588f 100644 --- a/lib/easy_stalk/job.rb +++ b/lib/easy_stalk/job.rb @@ -21,7 +21,7 @@ def self.tube_prefix(prefix=nil) if prefix @tube_prefix = prefix else - @tube_prefix || EasyStalk.configuration.default_tube_prefix + fetch_attribute(:tube_prefix) end end @@ -30,7 +30,7 @@ def self.priority(pri=nil) if pri @priority = pri else - @priority || EasyStalk.configuration.default_priority + fetch_attribute(:priority) end end @@ -39,7 +39,7 @@ def self.time_to_run(seconds=nil) if seconds @time_to_run = seconds else - @time_to_run || EasyStalk.configuration.default_time_to_run + fetch_attribute(:time_to_run) end end @@ -48,7 +48,7 @@ def self.delay(seconds=nil) if seconds @delay = seconds else - @delay || EasyStalk.configuration.default_delay + fetch_attribute(:delay) end end @@ -57,7 +57,7 @@ def self.retry_times(attempts=nil) if attempts @retry_times = attempts else - @retry_times || EasyStalk.configuration.default_retry_times + fetch_attribute(:retry_times) end end @@ -65,7 +65,19 @@ def self.serializable_context_keys(*keys) if keys.size > 0 @serializable_context_keys = keys else - @serializable_context_keys || DEFAULT_SERIALIZABLE_CONTEXT_KEYS + fetch_attribute(:serializable_context_keys, DEFAULT_SERIALIZABLE_CONTEXT_KEYS) + end + end + + def self.fetch_attribute(attribute, default=nil) + if self.instance_variable_defined?("@#{attribute}".to_sym) + self.instance_variable_get("@#{attribute}".to_sym) + elsif superclass.respond_to?(attribute.to_sym) + superclass.send(attribute.to_sym) + elsif EasyStalk.configuration.respond_to?("default_#{attribute}".to_sym) + EasyStalk.configuration.send("default_#{attribute}".to_sym) + else + default end end diff --git a/spec/lib/easy_stalk/job_spec.rb b/spec/lib/easy_stalk/job_spec.rb index 80f1b28..4b981f4 100644 --- a/spec/lib/easy_stalk/job_spec.rb +++ b/spec/lib/easy_stalk/job_spec.rb @@ -3,15 +3,24 @@ describe EasyStalk::Job do - class EasyStalk::MockJob < EasyStalk::Job + before do + class EasyStalk::MockJob < EasyStalk::Job + end + end + + after do + EasyStalk.send(:remove_const, :MockJob) end - describe EasyStalk::MockJob do + + subject { EasyStalk::MockJob.new } + + context "with a MockJob" do after do EasyStalk.configure end describe 'self << class' do - subject { described_class } + subject { EasyStalk::MockJob } describe '.tube_name' do it 'defaults to class name' do @@ -24,6 +33,7 @@ class MockJobWithName < subject end EasyStalk.configure { |config| config.default_tube_prefix = "rating.test." } expect(MockJobWithName.new.class.tube_name).to eq "rating.test.bar" + Object.send(:remove_const, :MockJobWithName) end it 'properly uses a prefix' do class MockJobWithNameAndPrefix < subject @@ -31,6 +41,7 @@ class MockJobWithNameAndPrefix < subject tube_prefix "foo." end expect(MockJobWithNameAndPrefix.new.class.tube_name).to eq "foo.bar" + Object.send(:remove_const, :MockJobWithNameAndPrefix) end end @@ -40,6 +51,7 @@ class MockJobWithPrefix < subject tube_prefix "bar." end expect(MockJobWithPrefix.new().class.tube_prefix).to eq "bar." + Object.send(:remove_const, :MockJobWithPrefix) end it 'uses the env if present' do EasyStalk.configure { |config| config.default_tube_prefix = "foo." } @@ -56,6 +68,7 @@ class MockJobWithPri < subject priority 25 end expect(MockJobWithPri.new().class.priority).to eq 25 + Object.send(:remove_const, :MockJobWithPri) end it 'uses default if not set' do expect(subject.priority).to eq EasyStalk::Configuration::DEFAULT_PRI @@ -68,6 +81,7 @@ class MockJobWithTtr < subject time_to_run 90 end expect(MockJobWithTtr.new().class.time_to_run).to eq 90 + Object.send(:remove_const, :MockJobWithTtr) end it 'uses default if not set' do expect(subject.time_to_run).to eq EasyStalk::Configuration::DEFAULT_TTR @@ -80,6 +94,7 @@ class MockJobWithDelay < subject delay 5 end expect(MockJobWithDelay.new().class.delay).to eq 5 + Object.send(:remove_const, :MockJobWithDelay) end it 'uses default if not set' do expect(subject.delay).to eq EasyStalk::Configuration::DEFAULT_DELAY @@ -92,6 +107,7 @@ class MockJobWithRetryTimes < subject retry_times 5 end expect(MockJobWithRetryTimes.new().class.retry_times).to eq 5 + Object.send(:remove_const, :MockJobWithRetryTimes) end it 'uses default if not set' do expect(subject.retry_times).to eq EasyStalk::Configuration::DEFAULT_RETRY_TIMES @@ -104,6 +120,7 @@ class MockJobWithKeys < subject serializable_context_keys :cat, :dog end expect(MockJobWithKeys.new().class.serializable_context_keys).to eq [:cat, :dog] + Object.send(:remove_const, :MockJobWithKeys) end it 'uses default if not set' do expect(subject.serializable_context_keys).to eq described_class::DEFAULT_SERIALIZABLE_CONTEXT_KEYS @@ -169,6 +186,7 @@ class MockJobWithKeys < described_class } MockJobWithKeys.new(:cat => "mew", "dog" => "wuf", :fish => "blu"). enqueue(conn, priority: pri, time_to_run: ttr, delay: delay) + Object.send(:remove_const, :MockJobWithKeys) end end @@ -195,13 +213,21 @@ class MockJobWithKeys < described_class end context = { :cat => "mew", "dog" => "wuf", :fish => "blu" } expect(MockJobWithKeys.new(context).job_data).to eq "{\"cat\":\"mew\",\"dog\":\"wuf\"}" + Object.send(:remove_const, :MockJobWithKeys) end end describe '.call' do - class ImplementedJob < described_class - def call; end + before do + class ImplementedJob < described_class + def call; end + end + end + + after do + Object.send(:remove_const, :ImplementedJob) end + it { expect { described_class.call }.to raise_error(NotImplementedError) } it { expect { ImplementedJob.call }.to_not raise_error } end From 15fd0a8321de21dcc209000e09e774b30465a52d Mon Sep 17 00:00:00 2001 From: Jake Epstein Date: Mon, 8 May 2017 22:14:28 +0000 Subject: [PATCH 2/6] added more tests around inheriting paramters for jobs --- spec/lib/easy_stalk/job_spec.rb | 71 +++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/spec/lib/easy_stalk/job_spec.rb b/spec/lib/easy_stalk/job_spec.rb index 4b981f4..367391a 100644 --- a/spec/lib/easy_stalk/job_spec.rb +++ b/spec/lib/easy_stalk/job_spec.rb @@ -43,6 +43,17 @@ class MockJobWithNameAndPrefix < subject expect(MockJobWithNameAndPrefix.new.class.tube_name).to eq "foo.bar" Object.send(:remove_const, :MockJobWithNameAndPrefix) end + it 'properly uses an inherited prefix' do + class MockJobWithName < subject + tube_prefix "foo." + end + class MockJobWithNameAndPrefix < MockJobWithName + tube_name "bar" + end + expect(MockJobWithNameAndPrefix.new.class.tube_name).to eq "foo.bar" + Object.send(:remove_const, :MockJobWithName) + Object.send(:remove_const, :MockJobWithNameAndPrefix) + end end describe '.tube_prefix' do @@ -53,6 +64,16 @@ class MockJobWithPrefix < subject expect(MockJobWithPrefix.new().class.tube_prefix).to eq "bar." Object.send(:remove_const, :MockJobWithPrefix) end + it 'uses inheritance if present' do + class MockJobWithPrefix < subject + tube_prefix "bar." + end + class MockChildJobWithPrefix < MockJobWithPrefix + end + expect(MockChildJobWithPrefix.new().class.tube_prefix).to eq "bar." + Object.send(:remove_const, :MockJobWithPrefix) + Object.send(:remove_const, :MockChildJobWithPrefix) + end it 'uses the env if present' do EasyStalk.configure { |config| config.default_tube_prefix = "foo." } expect(subject.tube_prefix).to eq "foo." @@ -70,6 +91,16 @@ class MockJobWithPri < subject expect(MockJobWithPri.new().class.priority).to eq 25 Object.send(:remove_const, :MockJobWithPri) end + it 'uses inheritance if present' do + class MockJobWithPri < subject + priority 25 + end + class MockChildJobWithPri < MockJobWithPri + end + expect(MockChildJobWithPri.new().class.priority).to eq 25 + Object.send(:remove_const, :MockJobWithPri) + Object.send(:remove_const, :MockChildJobWithPri) + end it 'uses default if not set' do expect(subject.priority).to eq EasyStalk::Configuration::DEFAULT_PRI end @@ -83,6 +114,16 @@ class MockJobWithTtr < subject expect(MockJobWithTtr.new().class.time_to_run).to eq 90 Object.send(:remove_const, :MockJobWithTtr) end + it 'uses inheritance if present' do + class MockJobWithTtr < subject + time_to_run 90 + end + class MockChildJobWithTtr < MockJobWithTtr + end + expect(MockChildJobWithTtr.new().class.time_to_run).to eq 90 + Object.send(:remove_const, :MockJobWithTtr) + Object.send(:remove_const, :MockChildJobWithTtr) + end it 'uses default if not set' do expect(subject.time_to_run).to eq EasyStalk::Configuration::DEFAULT_TTR end @@ -96,6 +137,16 @@ class MockJobWithDelay < subject expect(MockJobWithDelay.new().class.delay).to eq 5 Object.send(:remove_const, :MockJobWithDelay) end + it 'uses inheritance if present' do + class MockJobWithDelay < subject + delay 5 + end + class MockChildJobWithDelay < MockJobWithDelay + end + expect(MockChildJobWithDelay.new().class.delay).to eq 5 + Object.send(:remove_const, :MockJobWithDelay) + Object.send(:remove_const, :MockChildJobWithDelay) + end it 'uses default if not set' do expect(subject.delay).to eq EasyStalk::Configuration::DEFAULT_DELAY end @@ -109,6 +160,16 @@ class MockJobWithRetryTimes < subject expect(MockJobWithRetryTimes.new().class.retry_times).to eq 5 Object.send(:remove_const, :MockJobWithRetryTimes) end + it 'uses inheritance if present' do + class MockJobWithRetryTimes < subject + retry_times 5 + end + class MockChildJobWithRetryTimes < MockJobWithRetryTimes + end + expect(MockChildJobWithRetryTimes.new().class.retry_times).to eq 5 + Object.send(:remove_const, :MockJobWithRetryTimes) + Object.send(:remove_const, :MockChildJobWithRetryTimes) + end it 'uses default if not set' do expect(subject.retry_times).to eq EasyStalk::Configuration::DEFAULT_RETRY_TIMES end @@ -122,6 +183,16 @@ class MockJobWithKeys < subject expect(MockJobWithKeys.new().class.serializable_context_keys).to eq [:cat, :dog] Object.send(:remove_const, :MockJobWithKeys) end + it 'uses inheritance if present' do + class MockJobWithKeys < subject + serializable_context_keys :cat, :dog + end + class MockChildJobWithKeys < MockJobWithKeys + end + expect(MockChildJobWithKeys.new().class.serializable_context_keys).to eq [:cat, :dog] + Object.send(:remove_const, :MockJobWithKeys) + Object.send(:remove_const, :MockChildJobWithKeys) + end it 'uses default if not set' do expect(subject.serializable_context_keys).to eq described_class::DEFAULT_SERIALIZABLE_CONTEXT_KEYS end From f4f2f161e3ab4ce685eaf6664e0c972e251efbed Mon Sep 17 00:00:00 2001 From: Jake Epstein Date: Mon, 8 May 2017 22:26:31 +0000 Subject: [PATCH 3/6] added a test to confirm tube_name is not inherited --- spec/lib/easy_stalk/job_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/lib/easy_stalk/job_spec.rb b/spec/lib/easy_stalk/job_spec.rb index 367391a..d417c30 100644 --- a/spec/lib/easy_stalk/job_spec.rb +++ b/spec/lib/easy_stalk/job_spec.rb @@ -54,6 +54,16 @@ class MockJobWithNameAndPrefix < MockJobWithName Object.send(:remove_const, :MockJobWithName) Object.send(:remove_const, :MockJobWithNameAndPrefix) end + it 'does not inherit tube_name' do + class MockJobWithName < subject + tube_name "bar" + end + class MockChildJobWithoutName < MockJobWithName + end + expect(MockChildJobWithoutName.new.class.tube_name).to eq "MockChildJobWithoutName" + Object.send(:remove_const, :MockJobWithName) + Object.send(:remove_const, :MockChildJobWithoutName) + end end describe '.tube_prefix' do From 8ca9d0c5918ef13b07a6822372962baa46ca88e9 Mon Sep 17 00:00:00 2001 From: Jake Epstein Date: Mon, 8 May 2017 22:32:03 +0000 Subject: [PATCH 4/6] made a private method to prevent abuse --- lib/easy_stalk/job.rb | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/lib/easy_stalk/job.rb b/lib/easy_stalk/job.rb index 383588f..3eddf59 100644 --- a/lib/easy_stalk/job.rb +++ b/lib/easy_stalk/job.rb @@ -69,18 +69,6 @@ def self.serializable_context_keys(*keys) end end - def self.fetch_attribute(attribute, default=nil) - if self.instance_variable_defined?("@#{attribute}".to_sym) - self.instance_variable_get("@#{attribute}".to_sym) - elsif superclass.respond_to?(attribute.to_sym) - superclass.send(attribute.to_sym) - elsif EasyStalk.configuration.respond_to?("default_#{attribute}".to_sym) - EasyStalk.configuration.send("default_#{attribute}".to_sym) - else - default - end - end - def enqueue(beanstalk_connection, priority: nil, time_to_run: nil, delay: nil, delay_until: nil) tube = beanstalk_connection.tubes[self.class.tube_name] pri = priority || self.class.priority @@ -103,6 +91,20 @@ def job_data def call raise NotImplementedError end + + private + + def self.fetch_attribute(attribute, default=nil) + if self.instance_variable_defined?("@#{attribute}".to_sym) + self.instance_variable_get("@#{attribute}".to_sym) + elsif superclass.respond_to?(attribute.to_sym) + superclass.send(attribute.to_sym) + elsif EasyStalk.configuration.respond_to?("default_#{attribute}".to_sym) + EasyStalk.configuration.send("default_#{attribute}".to_sym) + else + default + end + end end end From b9f06f91ad62027423278877a9d04e483ec59e05 Mon Sep 17 00:00:00 2001 From: Jake Epstein Date: Tue, 9 May 2017 01:23:01 +0000 Subject: [PATCH 5/6] use method instead of variable to store job configurations --- lib/easy_stalk/configuration.rb | 2 +- lib/easy_stalk/job.rb | 87 ++++++++++++++---------------- lib/easy_stalk/tasks.rb | 2 +- lib/easy_stalk/worker.rb | 7 +-- spec/lib/easy_stalk/job_spec.rb | 48 ++++++++--------- spec/lib/easy_stalk/worker_spec.rb | 14 +++-- 6 files changed, 75 insertions(+), 85 deletions(-) diff --git a/lib/easy_stalk/configuration.rb b/lib/easy_stalk/configuration.rb index 5f05c5a..b9a9442 100644 --- a/lib/easy_stalk/configuration.rb +++ b/lib/easy_stalk/configuration.rb @@ -28,7 +28,7 @@ def initialize log.progname = Module.nesting.last.name end @default_worker_on_fail = Proc.new { |job_class, job_body, ex| - EasyStalk.logger.error "Worker for #{job_class} on tube[#{job_class.tube_name}] failed #{ex.message}" + EasyStalk.logger.error "Worker for #{job_class} on tube[#{job_class.get_tube_name}] failed #{ex.message}" EasyStalk.logger.error ex.backtrace.join("\n") } diff --git a/lib/easy_stalk/job.rb b/lib/easy_stalk/job.rb index 3eddf59..1a5d09e 100644 --- a/lib/easy_stalk/job.rb +++ b/lib/easy_stalk/job.rb @@ -10,70 +10,75 @@ class Job DEFAULT_SERIALIZABLE_CONTEXT_KEYS = [] def self.tube_name(tube=nil) - if tube - @tube_name = tube - else - tube_prefix + (@tube_name || name.split('::').last) - end + @tube_name = tube + end + def self.get_tube_name + get_tube_prefix + (@tube_name || name.split('::').last) end def self.tube_prefix(prefix=nil) - if prefix - @tube_prefix = prefix - else - fetch_attribute(:tube_prefix) + define_singleton_method :get_tube_prefix do + prefix end end + def self.get_tube_prefix + EasyStalk.configuration.default_tube_prefix + end def self.priority(pri=nil) # integer < 2**32. 0 is highest - if pri - @priority = pri - else - fetch_attribute(:priority) + define_singleton_method :get_priority do + pri end end + def self.get_priority + EasyStalk.configuration.default_priority + end def self.time_to_run(seconds=nil) # integer seconds to run this job - if seconds - @time_to_run = seconds - else - fetch_attribute(:time_to_run) + define_singleton_method :get_time_to_run do + seconds end end + def self.get_time_to_run + EasyStalk.configuration.default_time_to_run + end def self.delay(seconds=nil) # integer seconds before job is in ready queue - if seconds - @delay = seconds - else - fetch_attribute(:delay) + define_singleton_method :get_delay do + seconds end end + def self.get_delay + EasyStalk.configuration.default_delay + end def self.retry_times(attempts=nil) # max number of times to retry job before burying - if attempts - @retry_times = attempts - else - fetch_attribute(:retry_times) + define_singleton_method :get_retry_times do + attempts end end + def self.get_retry_times + EasyStalk.configuration.default_retry_times + end def self.serializable_context_keys(*keys) - if keys.size > 0 - @serializable_context_keys = keys - else - fetch_attribute(:serializable_context_keys, DEFAULT_SERIALIZABLE_CONTEXT_KEYS) + define_singleton_method :get_serializable_context_keys do + keys end end + def self.get_serializable_context_keys + DEFAULT_SERIALIZABLE_CONTEXT_KEYS + end def enqueue(beanstalk_connection, priority: nil, time_to_run: nil, delay: nil, delay_until: nil) - tube = beanstalk_connection.tubes[self.class.tube_name] - pri = priority || self.class.priority - ttr = time_to_run || self.class.time_to_run - delay = delay || self.class.delay + tube = beanstalk_connection.tubes[self.class.get_tube_name] + pri = priority || self.class.get_priority + ttr = time_to_run || self.class.get_time_to_run + delay = delay || self.class.get_delay if delay_until && DateTime === delay_until days = delay_until - DateTime.now @@ -84,27 +89,13 @@ def enqueue(beanstalk_connection, priority: nil, time_to_run: nil, delay: nil, d end def job_data - data = context.to_h.select { |key, value| self.class.serializable_context_keys.include? key } + data = context.to_h.select { |key, value| self.class.get_serializable_context_keys.include? key } JSON.dump(data) end def call raise NotImplementedError end - - private - - def self.fetch_attribute(attribute, default=nil) - if self.instance_variable_defined?("@#{attribute}".to_sym) - self.instance_variable_get("@#{attribute}".to_sym) - elsif superclass.respond_to?(attribute.to_sym) - superclass.send(attribute.to_sym) - elsif EasyStalk.configuration.respond_to?("default_#{attribute}".to_sym) - EasyStalk.configuration.send("default_#{attribute}".to_sym) - else - default - end - end end end diff --git a/lib/easy_stalk/tasks.rb b/lib/easy_stalk/tasks.rb index 78c8a74..d6ac99c 100644 --- a/lib/easy_stalk/tasks.rb +++ b/lib/easy_stalk/tasks.rb @@ -10,7 +10,7 @@ tubes = if args.extras.size > 0 EasyStalk::Job.descendants.select do |job| - tubs.include?(job.tube_name) + tubs.include?(job.get_tube_name) end end diff --git a/lib/easy_stalk/worker.rb b/lib/easy_stalk/worker.rb index 8fd1e65..cc04743 100644 --- a/lib/easy_stalk/worker.rb +++ b/lib/easy_stalk/worker.rb @@ -23,7 +23,7 @@ def work(job_classes = nil, on_fail: nil) @cancelled = false tube_class_hash = Hash[ - job_classes.map { |cls| [cls.tube_name, cls] } + job_classes.map { |cls| [cls.get_tube_name, cls] } ] pool = EasyStalk::Client.create_worker_pool(tube_class_hash.keys) @@ -40,7 +40,8 @@ def work(job_classes = nil, on_fail: nil) rescue => ex # Job issued a failed context or raised an unhandled exception job_class = tube_class_hash[job.tube] - if job.stats.releases <= job_class.retry_times + + if job.stats.releases <= job_class.get_retry_times # Re-enqueue with stepped delay release_with_delay(job) else @@ -54,7 +55,7 @@ def work(job_classes = nil, on_fail: nil) end end - jobs_list = job_classes.map { |job_class| "#{job_class} on tube #{job_class.tube_name}" }.join(", ") + jobs_list = job_classes.map { |job_class| "#{job_class} on tube #{job_class.get_tube_name}" }.join(", ") EasyStalk.logger.info "Worker running #{jobs_list} has been stopped" end diff --git a/spec/lib/easy_stalk/job_spec.rb b/spec/lib/easy_stalk/job_spec.rb index d417c30..b22bc14 100644 --- a/spec/lib/easy_stalk/job_spec.rb +++ b/spec/lib/easy_stalk/job_spec.rb @@ -25,14 +25,14 @@ class EasyStalk::MockJob < EasyStalk::Job describe '.tube_name' do it 'defaults to class name' do EasyStalk.configure { |config| config.default_tube_prefix = "rating.test." } - expect(subject.tube_name).to eq "rating.test.MockJob" + expect(subject.get_tube_name).to eq "rating.test.MockJob" end it 'can be set manually' do class MockJobWithName < subject tube_name "bar" end EasyStalk.configure { |config| config.default_tube_prefix = "rating.test." } - expect(MockJobWithName.new.class.tube_name).to eq "rating.test.bar" + expect(MockJobWithName.new.class.get_tube_name).to eq "rating.test.bar" Object.send(:remove_const, :MockJobWithName) end it 'properly uses a prefix' do @@ -40,7 +40,7 @@ class MockJobWithNameAndPrefix < subject tube_name "bar" tube_prefix "foo." end - expect(MockJobWithNameAndPrefix.new.class.tube_name).to eq "foo.bar" + expect(MockJobWithNameAndPrefix.new.class.get_tube_name).to eq "foo.bar" Object.send(:remove_const, :MockJobWithNameAndPrefix) end it 'properly uses an inherited prefix' do @@ -50,7 +50,7 @@ class MockJobWithName < subject class MockJobWithNameAndPrefix < MockJobWithName tube_name "bar" end - expect(MockJobWithNameAndPrefix.new.class.tube_name).to eq "foo.bar" + expect(MockJobWithNameAndPrefix.new.class.get_tube_name).to eq "foo.bar" Object.send(:remove_const, :MockJobWithName) Object.send(:remove_const, :MockJobWithNameAndPrefix) end @@ -60,7 +60,7 @@ class MockJobWithName < subject end class MockChildJobWithoutName < MockJobWithName end - expect(MockChildJobWithoutName.new.class.tube_name).to eq "MockChildJobWithoutName" + expect(MockChildJobWithoutName.new.class.get_tube_name).to eq "MockChildJobWithoutName" Object.send(:remove_const, :MockJobWithName) Object.send(:remove_const, :MockChildJobWithoutName) end @@ -71,7 +71,7 @@ class MockChildJobWithoutName < MockJobWithName class MockJobWithPrefix < subject tube_prefix "bar." end - expect(MockJobWithPrefix.new().class.tube_prefix).to eq "bar." + expect(MockJobWithPrefix.new().class.get_tube_prefix).to eq "bar." Object.send(:remove_const, :MockJobWithPrefix) end it 'uses inheritance if present' do @@ -80,16 +80,16 @@ class MockJobWithPrefix < subject end class MockChildJobWithPrefix < MockJobWithPrefix end - expect(MockChildJobWithPrefix.new().class.tube_prefix).to eq "bar." + expect(MockChildJobWithPrefix.new().class.get_tube_prefix).to eq "bar." Object.send(:remove_const, :MockJobWithPrefix) Object.send(:remove_const, :MockChildJobWithPrefix) end it 'uses the env if present' do EasyStalk.configure { |config| config.default_tube_prefix = "foo." } - expect(subject.tube_prefix).to eq "foo." + expect(subject.get_tube_prefix).to eq "foo." end it 'uses blank if no env' do - expect(subject.tube_prefix).to eq "" + expect(subject.get_tube_prefix).to eq "" end end @@ -98,7 +98,7 @@ class MockChildJobWithPrefix < MockJobWithPrefix class MockJobWithPri < subject priority 25 end - expect(MockJobWithPri.new().class.priority).to eq 25 + expect(MockJobWithPri.new().class.get_priority).to eq 25 Object.send(:remove_const, :MockJobWithPri) end it 'uses inheritance if present' do @@ -107,12 +107,12 @@ class MockJobWithPri < subject end class MockChildJobWithPri < MockJobWithPri end - expect(MockChildJobWithPri.new().class.priority).to eq 25 + expect(MockChildJobWithPri.new().class.get_priority).to eq 25 Object.send(:remove_const, :MockJobWithPri) Object.send(:remove_const, :MockChildJobWithPri) end it 'uses default if not set' do - expect(subject.priority).to eq EasyStalk::Configuration::DEFAULT_PRI + expect(subject.get_priority).to eq EasyStalk::Configuration::DEFAULT_PRI end end @@ -121,7 +121,7 @@ class MockChildJobWithPri < MockJobWithPri class MockJobWithTtr < subject time_to_run 90 end - expect(MockJobWithTtr.new().class.time_to_run).to eq 90 + expect(MockJobWithTtr.new().class.get_time_to_run).to eq 90 Object.send(:remove_const, :MockJobWithTtr) end it 'uses inheritance if present' do @@ -130,12 +130,12 @@ class MockJobWithTtr < subject end class MockChildJobWithTtr < MockJobWithTtr end - expect(MockChildJobWithTtr.new().class.time_to_run).to eq 90 + expect(MockChildJobWithTtr.new().class.get_time_to_run).to eq 90 Object.send(:remove_const, :MockJobWithTtr) Object.send(:remove_const, :MockChildJobWithTtr) end it 'uses default if not set' do - expect(subject.time_to_run).to eq EasyStalk::Configuration::DEFAULT_TTR + expect(subject.get_time_to_run).to eq EasyStalk::Configuration::DEFAULT_TTR end end @@ -144,7 +144,7 @@ class MockChildJobWithTtr < MockJobWithTtr class MockJobWithDelay < subject delay 5 end - expect(MockJobWithDelay.new().class.delay).to eq 5 + expect(MockJobWithDelay.new().class.get_delay).to eq 5 Object.send(:remove_const, :MockJobWithDelay) end it 'uses inheritance if present' do @@ -153,12 +153,12 @@ class MockJobWithDelay < subject end class MockChildJobWithDelay < MockJobWithDelay end - expect(MockChildJobWithDelay.new().class.delay).to eq 5 + expect(MockChildJobWithDelay.new().class.get_delay).to eq 5 Object.send(:remove_const, :MockJobWithDelay) Object.send(:remove_const, :MockChildJobWithDelay) end it 'uses default if not set' do - expect(subject.delay).to eq EasyStalk::Configuration::DEFAULT_DELAY + expect(subject.get_delay).to eq EasyStalk::Configuration::DEFAULT_DELAY end end @@ -167,7 +167,7 @@ class MockChildJobWithDelay < MockJobWithDelay class MockJobWithRetryTimes < subject retry_times 5 end - expect(MockJobWithRetryTimes.new().class.retry_times).to eq 5 + expect(MockJobWithRetryTimes.new().class.get_retry_times).to eq 5 Object.send(:remove_const, :MockJobWithRetryTimes) end it 'uses inheritance if present' do @@ -176,12 +176,12 @@ class MockJobWithRetryTimes < subject end class MockChildJobWithRetryTimes < MockJobWithRetryTimes end - expect(MockChildJobWithRetryTimes.new().class.retry_times).to eq 5 + expect(MockChildJobWithRetryTimes.new().class.get_retry_times).to eq 5 Object.send(:remove_const, :MockJobWithRetryTimes) Object.send(:remove_const, :MockChildJobWithRetryTimes) end it 'uses default if not set' do - expect(subject.retry_times).to eq EasyStalk::Configuration::DEFAULT_RETRY_TIMES + expect(subject.get_retry_times).to eq EasyStalk::Configuration::DEFAULT_RETRY_TIMES end end @@ -190,7 +190,7 @@ class MockChildJobWithRetryTimes < MockJobWithRetryTimes class MockJobWithKeys < subject serializable_context_keys :cat, :dog end - expect(MockJobWithKeys.new().class.serializable_context_keys).to eq [:cat, :dog] + expect(MockJobWithKeys.new().class.get_serializable_context_keys).to eq [:cat, :dog] Object.send(:remove_const, :MockJobWithKeys) end it 'uses inheritance if present' do @@ -199,12 +199,12 @@ class MockJobWithKeys < subject end class MockChildJobWithKeys < MockJobWithKeys end - expect(MockChildJobWithKeys.new().class.serializable_context_keys).to eq [:cat, :dog] + expect(MockChildJobWithKeys.new().class.get_serializable_context_keys).to eq [:cat, :dog] Object.send(:remove_const, :MockJobWithKeys) Object.send(:remove_const, :MockChildJobWithKeys) end it 'uses default if not set' do - expect(subject.serializable_context_keys).to eq described_class::DEFAULT_SERIALIZABLE_CONTEXT_KEYS + expect(subject.get_serializable_context_keys).to eq described_class::DEFAULT_SERIALIZABLE_CONTEXT_KEYS end end end diff --git a/spec/lib/easy_stalk/worker_spec.rb b/spec/lib/easy_stalk/worker_spec.rb index 45bd27c..6cad112 100644 --- a/spec/lib/easy_stalk/worker_spec.rb +++ b/spec/lib/easy_stalk/worker_spec.rb @@ -20,9 +20,7 @@ context "with a valid job class" do before do class ValidJob < EasyStalk::Job - def self.tube_name - "job_tube" - end + tube_name "job_tube" def call end @@ -44,7 +42,7 @@ def call expect(Beaneater).to receive(:new).and_return beanstalk expect(beanstalk).to receive(:tubes).and_return(tubes).at_least(1).times expect(tubes).to receive(:watch!).with("job_tube") - sample_job = EasyStalk::MockBeaneater::TubeItem.new("{}", nil, nil, nil, job_instance.class.tube_name) + sample_job = EasyStalk::MockBeaneater::TubeItem.new("{}", nil, nil, nil, job_instance.class.get_tube_name) expect(tubes).to receive(:reserve) { @count ||= 0 if @count < 2 @@ -64,7 +62,7 @@ def call tubes = EasyStalk::MockBeaneater::Tubes.new expect(EzPool).to receive(:new).and_return mocked_client expect(beanstalk).to receive(:tubes).and_return(tubes).at_least(1).times - sample_job = EasyStalk::MockBeaneater::TubeItem.new("{}", nil, nil, nil, job_instance.class.tube_name) + sample_job = EasyStalk::MockBeaneater::TubeItem.new("{}", nil, nil, nil, job_instance.class.get_tube_name) expect(tubes).to receive(:reserve) { @count ||= 0 if @count < 2 @@ -90,7 +88,7 @@ def call tubes = EasyStalk::MockBeaneater::Tubes.new expect(EzPool).to receive(:new).and_return mocked_client expect(beanstalk).to receive(:tubes).and_return(tubes).at_least(1).times - sample_job = EasyStalk::MockBeaneater::TubeItem.new("{}", nil, nil, nil, job_instance.class.tube_name) + sample_job = EasyStalk::MockBeaneater::TubeItem.new("{}", nil, nil, nil, job_instance.class.get_tube_name) expect(tubes).to receive(:reserve) { @count ||= 0 if @count < 2 @@ -129,7 +127,7 @@ def call tubes.watch!(ValidJob) expect(EzPool).to receive(:new).and_return mocked_client expect(beanstalk).to receive(:tubes).and_return(tubes).at_least(1).times - sample_job = EasyStalk::MockBeaneater::TubeItem.new("{}", nil, nil, nil, job_instance.class.tube_name) + sample_job = EasyStalk::MockBeaneater::TubeItem.new("{}", nil, nil, nil, job_instance.class.get_tube_name) expect(tubes).to receive(:reserve) { @count ||= 0 if @count < 2 @@ -172,7 +170,7 @@ def call subject.send :cleanup nil else - job = EasyStalk::MockBeaneater::TubeItem.new("{}", nil, nil, nil, ValidJob.tube_name, @count) + job = EasyStalk::MockBeaneater::TubeItem.new("{}", nil, nil, nil, ValidJob.get_tube_name, @count) expect(job).to receive(:bury) if @count == 3 job end From 28cda2f45c410dc93dd2c353c14d735d6b75dd74 Mon Sep 17 00:00:00 2001 From: Jake Epstein Date: Wed, 10 May 2017 19:11:38 +0000 Subject: [PATCH 6/6] use instance-instance rather than class-instance methods --- lib/easy_stalk/job.rb | 44 ++++++++++++++++----------------- spec/lib/easy_stalk/job_spec.rb | 24 +++++++++--------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/lib/easy_stalk/job.rb b/lib/easy_stalk/job.rb index 1a5d09e..0816119 100644 --- a/lib/easy_stalk/job.rb +++ b/lib/easy_stalk/job.rb @@ -25,60 +25,60 @@ def self.get_tube_prefix EasyStalk.configuration.default_tube_prefix end + def self.retry_times(attempts=nil) + # max number of times to retry job before burying + define_singleton_method :get_retry_times do + attempts + end + end + def self.get_retry_times + EasyStalk.configuration.default_retry_times + end + def self.priority(pri=nil) # integer < 2**32. 0 is highest - define_singleton_method :get_priority do + define_method :priority do pri end end - def self.get_priority + def priority EasyStalk.configuration.default_priority end def self.time_to_run(seconds=nil) # integer seconds to run this job - define_singleton_method :get_time_to_run do + define_method :time_to_run do seconds end end - def self.get_time_to_run + def time_to_run EasyStalk.configuration.default_time_to_run end def self.delay(seconds=nil) # integer seconds before job is in ready queue - define_singleton_method :get_delay do + define_method :delay do seconds end end - def self.get_delay + def delay EasyStalk.configuration.default_delay end - def self.retry_times(attempts=nil) - # max number of times to retry job before burying - define_singleton_method :get_retry_times do - attempts - end - end - def self.get_retry_times - EasyStalk.configuration.default_retry_times - end - def self.serializable_context_keys(*keys) - define_singleton_method :get_serializable_context_keys do + define_method :serializable_context_keys do keys end end - def self.get_serializable_context_keys + def serializable_context_keys DEFAULT_SERIALIZABLE_CONTEXT_KEYS end def enqueue(beanstalk_connection, priority: nil, time_to_run: nil, delay: nil, delay_until: nil) tube = beanstalk_connection.tubes[self.class.get_tube_name] - pri = priority || self.class.get_priority - ttr = time_to_run || self.class.get_time_to_run - delay = delay || self.class.get_delay + pri = priority || self.priority + ttr = time_to_run || self.time_to_run + delay = delay || self.delay if delay_until && DateTime === delay_until days = delay_until - DateTime.now @@ -89,7 +89,7 @@ def enqueue(beanstalk_connection, priority: nil, time_to_run: nil, delay: nil, d end def job_data - data = context.to_h.select { |key, value| self.class.get_serializable_context_keys.include? key } + data = context.to_h.select { |key, value| self.serializable_context_keys.include? key } JSON.dump(data) end diff --git a/spec/lib/easy_stalk/job_spec.rb b/spec/lib/easy_stalk/job_spec.rb index b22bc14..3dc080f 100644 --- a/spec/lib/easy_stalk/job_spec.rb +++ b/spec/lib/easy_stalk/job_spec.rb @@ -98,7 +98,7 @@ class MockChildJobWithPrefix < MockJobWithPrefix class MockJobWithPri < subject priority 25 end - expect(MockJobWithPri.new().class.get_priority).to eq 25 + expect(MockJobWithPri.new().priority).to eq 25 Object.send(:remove_const, :MockJobWithPri) end it 'uses inheritance if present' do @@ -107,12 +107,12 @@ class MockJobWithPri < subject end class MockChildJobWithPri < MockJobWithPri end - expect(MockChildJobWithPri.new().class.get_priority).to eq 25 + expect(MockChildJobWithPri.new().priority).to eq 25 Object.send(:remove_const, :MockJobWithPri) Object.send(:remove_const, :MockChildJobWithPri) end it 'uses default if not set' do - expect(subject.get_priority).to eq EasyStalk::Configuration::DEFAULT_PRI + expect(subject.new().priority).to eq EasyStalk::Configuration::DEFAULT_PRI end end @@ -121,7 +121,7 @@ class MockChildJobWithPri < MockJobWithPri class MockJobWithTtr < subject time_to_run 90 end - expect(MockJobWithTtr.new().class.get_time_to_run).to eq 90 + expect(MockJobWithTtr.new().time_to_run).to eq 90 Object.send(:remove_const, :MockJobWithTtr) end it 'uses inheritance if present' do @@ -130,12 +130,12 @@ class MockJobWithTtr < subject end class MockChildJobWithTtr < MockJobWithTtr end - expect(MockChildJobWithTtr.new().class.get_time_to_run).to eq 90 + expect(MockChildJobWithTtr.new().time_to_run).to eq 90 Object.send(:remove_const, :MockJobWithTtr) Object.send(:remove_const, :MockChildJobWithTtr) end it 'uses default if not set' do - expect(subject.get_time_to_run).to eq EasyStalk::Configuration::DEFAULT_TTR + expect(subject.new().time_to_run).to eq EasyStalk::Configuration::DEFAULT_TTR end end @@ -144,7 +144,7 @@ class MockChildJobWithTtr < MockJobWithTtr class MockJobWithDelay < subject delay 5 end - expect(MockJobWithDelay.new().class.get_delay).to eq 5 + expect(MockJobWithDelay.new().delay).to eq 5 Object.send(:remove_const, :MockJobWithDelay) end it 'uses inheritance if present' do @@ -153,12 +153,12 @@ class MockJobWithDelay < subject end class MockChildJobWithDelay < MockJobWithDelay end - expect(MockChildJobWithDelay.new().class.get_delay).to eq 5 + expect(MockChildJobWithDelay.new().delay).to eq 5 Object.send(:remove_const, :MockJobWithDelay) Object.send(:remove_const, :MockChildJobWithDelay) end it 'uses default if not set' do - expect(subject.get_delay).to eq EasyStalk::Configuration::DEFAULT_DELAY + expect(subject.new().delay).to eq EasyStalk::Configuration::DEFAULT_DELAY end end @@ -190,7 +190,7 @@ class MockChildJobWithRetryTimes < MockJobWithRetryTimes class MockJobWithKeys < subject serializable_context_keys :cat, :dog end - expect(MockJobWithKeys.new().class.get_serializable_context_keys).to eq [:cat, :dog] + expect(MockJobWithKeys.new().serializable_context_keys).to eq [:cat, :dog] Object.send(:remove_const, :MockJobWithKeys) end it 'uses inheritance if present' do @@ -199,12 +199,12 @@ class MockJobWithKeys < subject end class MockChildJobWithKeys < MockJobWithKeys end - expect(MockChildJobWithKeys.new().class.get_serializable_context_keys).to eq [:cat, :dog] + expect(MockChildJobWithKeys.new().serializable_context_keys).to eq [:cat, :dog] Object.send(:remove_const, :MockJobWithKeys) Object.send(:remove_const, :MockChildJobWithKeys) end it 'uses default if not set' do - expect(subject.get_serializable_context_keys).to eq described_class::DEFAULT_SERIALIZABLE_CONTEXT_KEYS + expect(subject.new().serializable_context_keys).to eq described_class::DEFAULT_SERIALIZABLE_CONTEXT_KEYS end end end