fix(digests): ensure consistent digests (#743)

* fix: consistent digests

* chore(lint): lint'em real good

* fix(redis): silence deprecations

* fix(spec): another wrong digest

* chore(deps): ignore sidekiq develop for now

* chore(deps): stupid compatibility

* fix(redis): use the ruby driver

* chore(deps): lock redis to < 5.0

.fasterer.yml

@@ -21,3 +21,6 @@ speedups:
 
 exclude_paths:
   - 'spec/**/*.rb'
+  - 'vendor/**/*.rb'
+  - 'gemfiles/**/*.rb'
+

Appraisals

@@ -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"

lib/sidekiq_unique_jobs/lock_digest.rb

@@ -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
 

sidekiq-unique-jobs.gemspec

@@ -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",

spec/sidekiq/api_spec.rb

@@ -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

spec/sidekiq_unique_jobs/digests_spec.rb

@@ -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
 
@@ -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)

spec/sidekiq_unique_jobs/job_spec.rb

@@ -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",
         },
       )

spec/sidekiq_unique_jobs/lock/base_lock_spec.rb

@@ -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

spec/sidekiq_unique_jobs/lua/reap_orphans_spec.rb

@@ -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)

spec/sidekiq_unique_jobs/middleware/client_spec.rb

@@ -9,7 +9,7 @@
       expect(schedule_count).to eq(1)
 
       expected = %w[
-        uniquejobs:26a33855311a0a653c5e0b4af1d1458d
+        uniquejobs:ae503400be25bde0465cebd14f6f0daf
       ]
 
       expect(keys).to include(*expected)

spec/sidekiq_unique_jobs/middleware/server_spec.rb

@@ -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")
 

spec/sidekiq_unique_jobs/on_conflict/reject_spec.rb

@@ -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",

spec/sidekiq_unique_jobs/on_conflict/replace_spec.rb

@@ -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 }
 

spec/sidekiq_unique_jobs/orphans/reaper_spec.rb

@@ -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)

spec/sidekiq_unique_jobs/sidekiq_worker_methods_spec.rb

@@ -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)

spec/sidekiq_unique_jobs/upgrade_locks_spec.rb

@@ -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)

spec/spec_helper.rb

@@ -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
@@ -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
@@ -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
 

spec/support/sidekiq_unique_jobs/testing.rb

@@ -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)