Skip to content

Commit

Permalink
Isolate the "schedules_changed" key
Browse files Browse the repository at this point in the history
Most other keys are centralized in `SidekiqScheduler::RedisManager` as class methods. This does the same with the "schedules_changed" key.
  • Loading branch information
TALlama authored and marcelolx committed Sep 8, 2022
1 parent 7342e72 commit 88db34a
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 12 deletions.
13 changes: 10 additions & 3 deletions lib/sidekiq-scheduler/redis_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,19 +106,19 @@ def self.schedule_exist?
#
# @return [Array] array with all the changed job names
def self.get_schedule_changes(from, to)
Sidekiq.redis { |r| r.zrangebyscore('schedules_changed', from, "(#{to}") }
Sidekiq.redis { |r| r.zrangebyscore(schedules_changed_key, from, "(#{to}") }
end

# Register a schedule change for a given job
#
# @param [String] name The name of the job
def self.add_schedule_change(name)
Sidekiq.redis { |r| r.zadd('schedules_changed', Time.now.to_f, name) }
Sidekiq.redis { |r| r.zadd(schedules_changed_key, Time.now.to_f, name) }
end

# Remove all the schedule changes records
def self.clean_schedules_changed
Sidekiq.redis { |r| r.del('schedules_changed') unless r.type('schedules_changed') == 'zset' }
Sidekiq.redis { |r| r.del(schedules_changed_key) unless r.type(schedules_changed_key) == 'zset' }
end

# Removes a queued job instance
Expand Down Expand Up @@ -187,6 +187,13 @@ def self.schedules_key
'schedules'
end

# Returns the Redis's key for saving schedule changes.
#
# @return [String] with the key
def self.schedules_changed_key
'schedules_changed'
end

private

# Returns the value of a Redis stored hash field
Expand Down
22 changes: 14 additions & 8 deletions spec/sidekiq-scheduler/redis_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@
let(:job_three_changed_time) { current_time - (2 * 60) }

before do
SidekiqScheduler::Store.zadd(:schedules_changed, job_one_changed_time.to_f, job_one)
SidekiqScheduler::Store.zadd(:schedules_changed, job_two_changed_time.to_f, job_two)
SidekiqScheduler::Store.zadd(:schedules_changed, job_three_changed_time.to_f, job_three)
SidekiqScheduler::Store.zadd(SidekiqScheduler::RedisManager.schedules_changed_key, job_one_changed_time.to_f, job_one)
SidekiqScheduler::Store.zadd(SidekiqScheduler::RedisManager.schedules_changed_key, job_two_changed_time.to_f, job_two)
SidekiqScheduler::Store.zadd(SidekiqScheduler::RedisManager.schedules_changed_key, job_three_changed_time.to_f, job_three)
end

it 'should return all changed jobs names in the range' do
Expand Down Expand Up @@ -248,7 +248,7 @@
Timecop.freeze(Time.now) do
subject

stored_schedules_changes = SidekiqScheduler::Store.zrangebyscore('schedules_changed', Time.now.to_f, Time.now.to_f)
stored_schedules_changes = SidekiqScheduler::Store.zrangebyscore(SidekiqScheduler::RedisManager.schedules_changed_key, Time.now.to_f, Time.now.to_f)
expect(stored_schedules_changes).to match_array(%w[some_job])
end
end
Expand All @@ -257,24 +257,24 @@
describe '.clean_schedules_changed' do
subject { described_class.clean_schedules_changed }

before { SidekiqScheduler::Store.zadd('schedules_changed', Time.now.to_f, 'some_job') }
before { SidekiqScheduler::Store.zadd(SidekiqScheduler::RedisManager.schedules_changed_key, Time.now.to_f, 'some_job') }

it "shouldn't remove the schedules_changed if it's sorted set" do
subject

expect(SidekiqScheduler::Store.exists?('schedules_changed')).to be true
expect(SidekiqScheduler::Store.exists?(SidekiqScheduler::RedisManager.schedules_changed_key)).to be true
end

context 'when schedules_changed is not a sorted set' do
before do
SidekiqScheduler::Store.clean
SidekiqScheduler::Store.sadd('schedules_changed', 'some_job')
SidekiqScheduler::Store.sadd(SidekiqScheduler::RedisManager.schedules_changed_key, 'some_job')
end

it 'should remove the schedules_changed set' do
subject

expect(SidekiqScheduler::Store.exists?('schedules_changed')).to be false
expect(SidekiqScheduler::Store.exists?(SidekiqScheduler::RedisManager.schedules_changed_key)).to be false
end
end
end
Expand Down Expand Up @@ -368,4 +368,10 @@

it { is_expected.to eq('schedules') }
end

describe ".schedules_changed_key" do
subject { described_class.schedules_changed_key }

it { is_expected.to eq('schedules_changed') }
end
end
2 changes: 1 addition & 1 deletion spec/support/store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def self.job_from_redis(job_id)
end

def self.changed_job?(job_id)
Sidekiq.redis { |redis| !!redis.zrank('schedules_changed', job_id) }
Sidekiq.redis { |redis| !!redis.zrank(SidekiqScheduler::RedisManager.schedules_changed_key, job_id) }
end

def self.job_from_redis_without_decoding(job_id)
Expand Down

0 comments on commit 88db34a

Please sign in to comment.