Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable replace strategy #315

Merged
merged 7 commits into from
Aug 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions .reek.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,26 @@ detectors:
- Array
NilCheck:
enabled: false
DataClump:
enabled: true
exclude:
- SidekiqUniqueJobs::Util
FeatureEnvy:
exclude:
- SidekiqUniqueJobs::OnConflict::Reject#push_to_deadset
- SidekiqUniqueJobs::Digests#batch_delete
- SidekiqUniqueJobs::Digests#page
- SidekiqUniqueJobs::Logging#debug_item
- SidekiqUniqueJobs::OnConflict::Reject#push_to_deadset
- SidekiqUniqueJobs::Util#batch_delete
- SidekiqUniqueJobs::Digests#batch_delete
- SidekiqUniqueJobs::Web::Helpers#cparams
NestedIterators:
exclude:
- SidekiqUniqueJobs::Digests#batch_delete
- SidekiqUniqueJobs::Locksmith#create_lock
- SidekiqUniqueJobs::Middleware#configure_client_middleware
- SidekiqUniqueJobs::Middleware#configure_server_middleware
- SidekiqUniqueJobs::Util#batch_delete
- SidekiqUniqueJobs::Digests#batch_delete
- SidekiqUniqueJobs::Util#keys_with_ttl
TooManyInstanceVariables:
exclude:
- SidekiqUniqueJobs::Locksmith
Expand Down
1 change: 1 addition & 0 deletions lib/sidekiq_unique_jobs/lock/base_lock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def initialize(item, callback, redis_pool = nil)
# @return [String] the sidekiq job id
def lock
@attempt = 0
return item[JID_KEY] if locked?

if (token = locksmith.lock(item[LOCK_TIMEOUT_KEY]))
token
Expand Down
1 change: 1 addition & 0 deletions lib/sidekiq_unique_jobs/on_conflict.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module OnConflict
log: OnConflict::Log,
raise: OnConflict::Raise,
reject: OnConflict::Reject,
replace: OnConflict::Replace,
reschedule: OnConflict::Reschedule,
}.freeze

Expand Down
2 changes: 1 addition & 1 deletion lib/sidekiq_unique_jobs/on_conflict/replace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module SidekiqUniqueJobs
module OnConflict
# Strategy to raise an error on conflict
# Strategy to replace the job on conflict
#
# @author Mikael Henriksson <[email protected]>
class Replace < OnConflict::Strategy
Expand Down
14 changes: 14 additions & 0 deletions lib/sidekiq_unique_jobs/util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,20 @@ def keys(pattern = SCAN_PATTERN, count = DEFAULT_COUNT)
redis { |conn| conn.scan_each(match: prefix(pattern), count: count).to_a }
end

# Find unique keys with ttl
# @param [String] pattern a pattern to scan for in redis
# @param [Integer] count the maximum number of keys to delete
# @return [Hash<String, Integer>] a hash with active unique keys and corresponding ttl
def keys_with_ttl(pattern = SCAN_PATTERN, count = DEFAULT_COUNT)
hash = {}
redis do |conn|
conn.scan_each(match: prefix(pattern), count: count).each do |key|
hash[key] = conn.ttl(key)
end
end
hash
end

# Deletes unique keys from redis
#
# @param [String] pattern a pattern to scan for in redis
Expand Down
85 changes: 85 additions & 0 deletions spec/integration/sidekiq_unique_jobs/lock/until_executing_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# frozen_string_literal: true

require 'spec_helper'

RSpec.describe SidekiqUniqueJobs::Lock::UntilExecuting, redis: :redis do
include SidekiqHelpers

let(:process_one) { described_class.new(item_one, callback) }
let(:process_two) { described_class.new(item_two, callback) }

let(:jid_one) { 'jid one' }
let(:jid_two) { 'jid two' }
let(:worker_class) { UntilExecutedJob }
let(:unique) { :until_executed }
let(:queue) { :executed }
let(:args) { %w[array of arguments] }
let(:callback) { -> {} }
let(:item_one) do
{ 'jid' => jid_one,
'class' => worker_class.to_s,
'queue' => queue,
'lock' => unique,
'args' => args }
end
let(:item_two) do
{ 'jid' => jid_two,
'class' => worker_class.to_s,
'queue' => queue,
'lock' => unique,
'args' => args }
end

before do
allow(callback).to receive(:call).and_call_original
end

describe '#lock' do
it_behaves_like 'a lock implementation'
end

describe '#execute' do
it 'unlocks before executing' do
process_one.lock
process_one.execute do
expect(process_one.locked?).to eq(false)
end
end
end

describe '#delete' do
subject(:delete) { process_one.delete }

context 'when locked' do
context 'when expiration is not negative' do
it 'deletes the lock without fuss' do
worker_class.use_options(lock_expiration: nil) do
process_one.lock
expect { delete }.to change { unique_keys.size }.from(3).to(0)
end
end
end

context 'when expiration is positive' do
it 'does not delete the lock' do
worker_class.use_options(lock_expiration: 100) do
process_one.lock
expect { delete }.not_to change(unique_keys, :size)
end
end
end
end
end

describe '#delete!' do
subject(:delete!) { process_one.delete! }

context 'when locked' do
before { process_one.lock }

it 'deletes the lock without fuss' do
expect { delete! }.to change { unique_keys.size }.from(3).to(0)
end
end
end
end
29 changes: 20 additions & 9 deletions spec/unit/sidekiq_unique_jobs/lock/base_lock_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,31 @@ def execute
describe '#lock' do
subject(:lock_lock) { lock.lock }

before do
allow(locksmith).to receive(:lock).with(kind_of(Integer)).and_return(token)
context 'when already locked?' do
before do
allow(lock).to receive(:locked?).and_return(true)
end

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

context 'when a token is retrieved' do
let(:token) { 'another jid' }
context 'when not locked?' do
before do
allow(lock).to receive(:locked?).and_return(false)
allow(locksmith).to receive(:lock).with(kind_of(Integer)).and_return(token)
end

context 'when a token is retrieved' do
let(:token) { 'another jid' }

it { is_expected.to eq('another jid') }
end
it { is_expected.to eq('another jid') }
end

context 'when token is not retrieved' do
let(:token) { nil }
context 'when token is not retrieved' do
let(:token) { nil }

it { is_expected.to eq(nil) }
it { is_expected.to eq(nil) }
end
end
end

Expand Down
21 changes: 21 additions & 0 deletions spec/unit/sidekiq_unique_jobs/on_conflict_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# frozen_string_literal: true

require 'spec_helper'

RSpec.describe SidekiqUniqueJobs::OnConflict do
describe '::STRAGEGIES' do
subject { described_class::STRATEGIES }

let(:expected) do
{
log: described_class::Log,
raise: described_class::Raise,
reject: described_class::Reject,
replace: described_class::Replace,
reschedule: described_class::Reschedule,
}
end

it { is_expected.to eq(expected) }
end
end