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

fix(digests): ensure consistent digests #743

Merged
merged 9 commits into from
Dec 3, 2022
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
3 changes: 3 additions & 0 deletions .fasterer.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,6 @@ speedups:

exclude_paths:
- 'spec/**/*.rb'
- 'vendor/**/*.rb'
- 'gemfiles/**/*.rb'

6 changes: 3 additions & 3 deletions Appraisals
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# frozen_string_literal: true

appraise "sidekiq-develop" do
gem "sidekiq", git: "https://github.com/mperham/sidekiq.git"
end
# appraise "sidekiq-develop" do
# gem "sidekiq", git: "https://github.com/mperham/sidekiq.git"
# end

appraise "sidekiq-5.0" do
gem "sidekiq", "~> 5.0.0"
Expand Down
29 changes: 0 additions & 29 deletions gemfiles/sidekiq_develop.gemfile

This file was deleted.

2 changes: 1 addition & 1 deletion lib/sidekiq_unique_jobs/lock_digest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def lock_digest
# Creates a namespaced unique digest based on the {#digestable_hash} and the {#lock_prefix}
# @return [String] a unique digest
def create_digest
digest = OpenSSL::Digest::MD5.hexdigest(dump_json(digestable_hash))
digest = OpenSSL::Digest::MD5.hexdigest(dump_json(digestable_hash.sort))
"#{lock_prefix}:#{digest}"
end

Expand Down
7 changes: 0 additions & 7 deletions myapp/spec/models/post_spec.rb

This file was deleted.

7 changes: 0 additions & 7 deletions myapp/spec/models/user_spec.rb

This file was deleted.

3 changes: 2 additions & 1 deletion sidekiq-unique-jobs.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ Gem::Specification.new do |spec|

spec.add_dependency "brpoplpush-redis_script", "> 0.1.1", "<= 2.0.0"
spec.add_dependency "concurrent-ruby", "~> 1.0", ">= 1.0.5"
spec.add_dependency "sidekiq", ">= 5.0", "< 8.0"
spec.add_dependency "redis", "< 5.0"
spec.add_dependency "sidekiq", ">= 5.0", "< 7.0"
spec.add_dependency "thor", ">= 0.20", "< 3.0"
spec.metadata = {
"rubygems_mfa_required" => "true",
Expand Down
2 changes: 1 addition & 1 deletion spec/sidekiq/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"queue" => "testqueue",
"args" => [{ foo: "bar" }] }
end
let(:lock_digest) { "uniquejobs:577db3c4fc72230bf2c256faa708a083" }
let(:lock_digest) { "uniquejobs:1fa0458e74c76c7029b1f4f00bc59ec9" }
let(:key) { SidekiqUniqueJobs::Key.new(lock_digest) }

describe Sidekiq::SortedEntry::UniqueExtension do
Expand Down
22 changes: 11 additions & 11 deletions spec/sidekiq_unique_jobs/digests_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@
shared_context "with a regular job" do
let(:expected_keys) do
{
"uniquejobs:0781b1f587a9a8d08773f21ed752caed" => kind_of(Float),
"uniquejobs:09b3a42f77a75865bf27ac44e66bb4ef" => kind_of(Float),
"uniquejobs:1c4c0bf2a8a1006c7610e4ef1f965f34" => kind_of(Float),
"uniquejobs:236feda848cfbb1ae32d4d9af666349e" => kind_of(Float),
"uniquejobs:77d1e23f18943bc048b48a01b85af0b3" => kind_of(Float),
"uniquejobs:9fcaaf3e873f101a8d79e00e89bb3b36" => kind_of(Float),
"uniquejobs:a47040c19b3741eaf912a96cd8bee728" => kind_of(Float),
"uniquejobs:c85f45d715232cfff0505fb85ca92659" => kind_of(Float),
"uniquejobs:cb5be91a66b83435281c23fe489f22b5" => kind_of(Float),
"uniquejobs:e74bebbf569d620397688fded62c85d6" => kind_of(Float),
"uniquejobs:191cc66e8db74a712ca80180d846a8c0" => kind_of(Float),
"uniquejobs:70091c2e18d6b45cd1a257a7b77c1dc0" => kind_of(Float),
"uniquejobs:72254d80583af0f3cf1ff3cd8271c532" => kind_of(Float),
"uniquejobs:7f1a663f444e9629ed73893541564351" => kind_of(Float),
"uniquejobs:a1d714a6dacd9fcfe0aa6274af3d5ab4" => kind_of(Float),
"uniquejobs:b9d8a7667ef91f07e9c5a735e08e0891" => kind_of(Float),
"uniquejobs:ced55fba57e2d67b2422cacbe08896f4" => kind_of(Float),
"uniquejobs:e284198d560db35309c4d1b49e325645" => kind_of(Float),
"uniquejobs:e3544c3ca5a5b28141a1d161c70d04cb" => kind_of(Float),
"uniquejobs:eb82e9e8057a8912a3f970c8768975f7" => kind_of(Float),
}
end

Expand Down Expand Up @@ -73,7 +73,7 @@
context "with a regular job" do
include_context "with a regular job"

let(:digest) { "uniquejobs:e74bebbf569d620397688fded62c85d6" }
let(:digest) { "uniquejobs:a1d714a6dacd9fcfe0aa6274af3d5ab4" }

before do
allow(digests).to receive(:log_info)
Expand Down
2 changes: 1 addition & 1 deletion spec/sidekiq_unique_jobs/job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"lock_timeout" => MyUniqueJob.get_sidekiq_options["lock_timeout"].to_i,
"lock_ttl" => MyUniqueJob.get_sidekiq_options["lock_ttl"],
"lock_args" => args,
"lock_digest" => "uniquejobs:149ef752a5776e0bd05929b8f0e33250",
"lock_digest" => "uniquejobs:6b6835a019cad7c2a7a4e53e20a9184c",
"lock_prefix" => "uniquejobs",
},
)
Expand Down
5 changes: 1 addition & 4 deletions spec/sidekiq_unique_jobs/lock/base_lock_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,7 @@
allow(lock).to receive(:reflect)
end

it "reflects" do
end

it "logs a warning" do
it "reflects a warning" do
expect { callback_safely }.to raise_error(RuntimeError, "Hell")
expect(lock).to have_received(:reflect).with(:after_unlock_callback_failed, item)
end
Expand Down
6 changes: 5 additions & 1 deletion spec/sidekiq_unique_jobs/lua/reap_orphans_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,11 @@
before do
SidekiqUniqueJobs.redis do |conn|
conn.multi do |pipeline|
pipeline.sadd("processes", process_key)
if pipeline.respond_to?(:sadd?)
pipeline.sadd?("processes", process_key)
else
pipeline.sadd("processes", process_key)
end
pipeline.hset(worker_key, thread_id, dump_json(item))
pipeline.expire(process_key, 60)
pipeline.expire(worker_key, 60)
Expand Down
2 changes: 1 addition & 1 deletion spec/sidekiq_unique_jobs/middleware/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
expect(schedule_count).to eq(1)

expected = %w[
uniquejobs:26a33855311a0a653c5e0b4af1d1458d
uniquejobs:ae503400be25bde0465cebd14f6f0daf
]

expect(keys).to include(*expected)
Expand Down
3 changes: 1 addition & 2 deletions spec/sidekiq_unique_jobs/middleware/server_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
it "does not unlock keys it does not own" do
jid = UntilExecutedJob.perform_async
item = Sidekiq::Queue.new(queue).find_job(jid).item

digest = "uniquejobs:cf51f14f752c9ca8f3cfb0bbebad4abc"
digest = "uniquejobs:41459093fde370420ea1d1f446b60281"
expect(get(digest)).to eq(jid)
set(digest, "NOT_DELETED")

Expand Down
2 changes: 1 addition & 1 deletion spec/sidekiq_unique_jobs/on_conflict/reject_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
include_context "with a stubbed locksmith"
let(:strategy) { described_class.new(item) }
let(:deadset) { instance_spy(Sidekiq::DeadSet) }
let(:payload) { instance_spy("payload") } # rubocop:disable RSpec/VerifiedDoubleReference
let(:payload) { instance_spy(String) }
let(:item) do
{ "jid" => "maaaahjid",
"class" => "WhileExecutingRejectJob",
Expand Down
2 changes: 1 addition & 1 deletion spec/sidekiq_unique_jobs/on_conflict/replace_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

RSpec.describe SidekiqUniqueJobs::OnConflict::Replace do
let(:strategy) { described_class.new(item) }
let(:lock_digest) { "uniquejobs:0781b1f587a9a8d08773f21ed752caed" }
let(:lock_digest) { "uniquejobs:a1d714a6dacd9fcfe0aa6274af3d5ab4" }
let(:block) { -> { p "Hello" } }
let(:digest) { digests.entries.first }

Expand Down
6 changes: 5 additions & 1 deletion spec/sidekiq_unique_jobs/orphans/reaper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,11 @@
before do
SidekiqUniqueJobs.redis do |conn|
conn.multi do |pipeline|
pipeline.sadd("processes", process_key)
if pipeline.respond_to?(:sadd?)
pipeline.sadd?("processes", process_key)
else
pipeline.sadd("processes", process_key)
end
pipeline.set(process_key, "bogus")
pipeline.hset(worker_key, thread_id, dump_json(payload: item.merge(created_at: created_at)))
pipeline.expire(process_key, 60)
Expand Down
1 change: 1 addition & 0 deletions spec/sidekiq_unique_jobs/sidekiq_worker_methods_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def initialize(job_class)
let(:error_message) { "this class does not exist" }

before do
allow(Object).to receive(:const_get).and_call_original
allow(Object).to receive(:const_get)
.with(job_class)
.and_raise(NameError, error_message)
Expand Down
6 changes: 5 additions & 1 deletion spec/sidekiq_unique_jobs/upgrade_locks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@
conn.pipelined do |pipeline|
chunk.each do |digest|
job_id = SecureRandom.hex(12)
pipeline.sadd("unique:keys", digest)
if pipeline.respond_to?(:sadd?)
pipeline.sadd?("unique:keys", digest)
else
pipeline.sadd("unique:keys", digest)
end
pipeline.set("#{digest}:EXISTS", job_id)
pipeline.rpush("#{digest}:AVAILABLE", digest)
pipeline.hset("#{digest}:GRABBED", job_id, now_f)
Expand Down
8 changes: 4 additions & 4 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
end

Sidekiq.configure_server do |config|
config.redis = { port: 6379, driver: :hiredis }
config.redis = { port: 6379 }

config.server_middleware do |chain|
chain.add SidekiqUniqueJobs::Middleware::Server
Expand All @@ -53,7 +53,7 @@
end

Sidekiq.configure_client do |config|
config.redis = { port: 6379, driver: :hiredis }
config.redis = { port: 6379 }

config.client_middleware do |chain|
chain.add SidekiqUniqueJobs::Middleware::Client
Expand Down Expand Up @@ -109,11 +109,11 @@

config.before do
Sidekiq.configure_server do |conf|
conf.redis = { port: 6379, driver: :hiredis }
conf.redis = { port: 6379 }
end

Sidekiq.configure_client do |conf|
conf.redis = { port: 6379, driver: :hiredis }
conf.redis = { port: 6379 }
end
end

Expand Down
8 changes: 7 additions & 1 deletion spec/support/sidekiq_unique_jobs/testing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,13 @@ def scard(key)
end

def sadd(key, member)
redis { |conn| conn.sadd(key, member) }
redis do |conn|
if conn.respond_to?(:sadd?)
conn.sadd?(key, member)
else
conn.sadd(key, member)
end
end
end

def srem(key, member)
Expand Down