From e46cdd67d72b3f951fddd8574ea31a8f32cc3675 Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Thu, 25 Jul 2024 14:50:19 +0200 Subject: [PATCH] feat(digest): allow modern algorithm --- .github/workflows/rspec.yml | 1 + .rubocop.yml | 8 +++- Appraisals | 4 ++ README.md | 35 +++++++++++----- gemfiles/sidekiq_7.1.gemfile | 2 +- gemfiles/sidekiq_7.2.gemfile | 2 +- gemfiles/sidekiq_7.3.gemfile | 28 +++++++++++++ lib/sidekiq_unique_jobs/config.rb | 24 +++++++++-- lib/sidekiq_unique_jobs/digests.rb | 2 +- .../lock/while_executing.rb | 2 +- lib/sidekiq_unique_jobs/lock_digest.rb | 7 +++- .../on_conflict/replace.rb | 2 +- .../on_conflict/reschedule.rb | 2 +- lib/sidekiq_unique_jobs/orphans/manager.rb | 3 +- .../orphans/ruby_reaper.rb | 2 +- lib/sidekiq_unique_jobs/sidekiq_unique_ext.rb | 4 +- lib/sidekiq_unique_jobs/testing.rb | 2 +- spec/performance/lock_digest_spec.rb | 4 +- spec/performance/locksmith_spec.rb | 4 +- spec/sidekiq_unique_jobs/cli_spec.rb | 6 +-- .../sidekiq_unique_jobs/configuration_spec.rb | 4 +- spec/sidekiq_unique_jobs/lock_digest_spec.rb | 40 ++++++++++++++----- .../server/until_and_while_executing_spec.rb | 4 +- .../script/scripts_spec.rb | 2 +- ...til_and_while_executing_reject_job_spec.rb | 4 +- ...il_and_while_executing_replace_job_spec.rb | 2 +- 26 files changed, 149 insertions(+), 51 deletions(-) create mode 100644 gemfiles/sidekiq_7.3.gemfile diff --git a/.github/workflows/rspec.yml b/.github/workflows/rspec.yml index bea0a85d8..e3a845eab 100644 --- a/.github/workflows/rspec.yml +++ b/.github/workflows/rspec.yml @@ -63,6 +63,7 @@ jobs: - sidekiq_7.0 - sidekiq_7.1 - sidekiq_7.2 + - sidekiq_7.3 steps: - uses: actions/checkout@v4 diff --git a/.rubocop.yml b/.rubocop.yml index 1eea75746..b06ff4809 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -115,7 +115,13 @@ RSpec/ExpectActual: RSpec/ExpectChange: EnforcedStyle: block -RSpec/FilePath: +RSpec/SpecFilePathFormat: + Enabled: true + Exclude: + - spec/performance/locksmith_spec.rb + - spec/performance/lock_digest_spec.rb + +RSpec/SpecFilePathSuffix: Enabled: true Exclude: - spec/performance/locksmith_spec.rb diff --git a/Appraisals b/Appraisals index cc63395ab..34d3512f5 100644 --- a/Appraisals +++ b/Appraisals @@ -15,3 +15,7 @@ end appraise "sidekiq-7.2" do gem "sidekiq", "~> 7.2.0" end + +appraise "sidekiq-7.3" do + gem "sidekiq", "~> 7.3.0" +end diff --git a/README.md b/README.md index ff3976fcd..358a65bd5 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,7 @@ Want to show me some ❤️ for the hard work I do on this gem? You can use the - [sidekiq-global_id](#sidekiq-global_id) - [sidekiq-status](#sidekiq-status) - [Global Configuration](#global-configuration) + - [digest_algorithm](#digest_algorithm) - [debug_lua](#debug_lua) - [lock_timeout](#lock_timeout) - [lock_ttl](#lock_ttl) @@ -734,17 +735,29 @@ Configure SidekiqUniqueJobs in an initializer or the sidekiq initializer on appl ```ruby SidekiqUniqueJobs.configure do |config| - config.logger = Sidekiq.logger # default, change at your own discretion - config.logger_enabled = true # default, disable for test environments - config.debug_lua = false # Turn on when debugging - config.lock_info = false # Turn on when debugging - config.lock_ttl = 600 # Expire locks after 10 minutes - config.lock_timeout = nil # turn off lock timeout - config.max_history = 0 # Turn on when debugging - config.reaper = :ruby # :ruby, :lua or :none/nil - config.reaper_count = 1000 # Stop reaping after this many keys - config.reaper_interval = 600 # Reap orphans every 10 minutes - config.reaper_timeout = 150 # Timeout reaper after 2.5 minutes + config.logger = Sidekiq.logger # default, change at your own discretion + config.logger_enabled = true # default, disable for test environments + config.debug_lua = false # Turn on when debugging + config.lock_info = false # Turn on when debugging + config.lock_ttl = 600 # Expire locks after 10 minutes + config.lock_timeout = nil # turn off lock timeout + config.max_history = 0 # Turn on when debugging + config.reaper = :ruby # :ruby, :lua or :none/nil + config.reaper_count = 1000 # Stop reaping after this many keys + config.reaper_interval = 600 # Reap orphans every 10 minutes + config.reaper_timeout = 150 # Timeout reaper after 2.5 minutes + config.digest_algorithm = :modern # Timeout reaper after 2.5 minutes +end +``` +#### digest_algorithm + +For backwards compatibility this one is set to `:legacy` by the default. If you happen to run into issues with FIPS being enabled on your redis server you might want to set this to `:modern`. + +See: https://github.com/mhenrixon/sidekiq-unique-jobs/issues/848 for explanation + +```ruby +SidekiqUniqueJobs.configure do |config| + config.digest_algorithm = :modern # Timeout reaper after 2.5 minutes end ``` diff --git a/gemfiles/sidekiq_7.1.gemfile b/gemfiles/sidekiq_7.1.gemfile index 140999215..539a3be9a 100644 --- a/gemfiles/sidekiq_7.1.gemfile +++ b/gemfiles/sidekiq_7.1.gemfile @@ -19,7 +19,7 @@ gem "sinatra" gem "timecop" gem "toxiproxy" gem "yard" -gem "sidekiq", "~> 7.0.0" +gem "sidekiq", "~> 7.1.0" platforms :mri do gem "concurrent-ruby-ext" diff --git a/gemfiles/sidekiq_7.2.gemfile b/gemfiles/sidekiq_7.2.gemfile index 140999215..be782364c 100644 --- a/gemfiles/sidekiq_7.2.gemfile +++ b/gemfiles/sidekiq_7.2.gemfile @@ -19,7 +19,7 @@ gem "sinatra" gem "timecop" gem "toxiproxy" gem "yard" -gem "sidekiq", "~> 7.0.0" +gem "sidekiq", "~> 7.2.0" platforms :mri do gem "concurrent-ruby-ext" diff --git a/gemfiles/sidekiq_7.3.gemfile b/gemfiles/sidekiq_7.3.gemfile new file mode 100644 index 000000000..bc1012eb1 --- /dev/null +++ b/gemfiles/sidekiq_7.3.gemfile @@ -0,0 +1,28 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "appraisal" +gem "faraday-retry" +gem "gem-release" +gem "github-markup" +gem "rack-test" +gem "rake", "13.0.3" +gem "reek", ">= 5.3" +gem "rspec" +gem "rspec-benchmark" +gem "rspec-html-matchers" +gem "rspec-its" +gem "rubocop-mhenrixon" +gem "simplecov-sublime", ">= 0.21.2", require: false +gem "sinatra" +gem "timecop" +gem "toxiproxy" +gem "yard" +gem "sidekiq", "~> 7.3.0" + +platforms :mri do + gem "concurrent-ruby-ext" +end + +gemspec path: "../" diff --git a/lib/sidekiq_unique_jobs/config.rb b/lib/sidekiq_unique_jobs/config.rb index 23192823c..bd1857821 100644 --- a/lib/sidekiq_unique_jobs/config.rb +++ b/lib/sidekiq_unique_jobs/config.rb @@ -21,7 +21,8 @@ module SidekiqUniqueJobs :reaper_resurrector_enabled, :lock_info, :raise_on_config_error, - :current_redis_version) + :current_redis_version, + :digest_algorithm) # # Shared class for dealing with gem configuration @@ -118,11 +119,9 @@ class Config < ThreadSafeConfig # # @return [3600] check if reaper is dead each 3600 seconds REAPER_RESURRECTOR_INTERVAL = 3600 - # # @return [false] enable reaper resurrector REAPER_RESURRECTOR_ENABLED = false - # # @return [false] while useful it also adds overhead so disable lock_info by default USE_LOCK_INFO = false @@ -132,6 +131,9 @@ class Config < ThreadSafeConfig # # @return [0.0.0] default redis version is only to avoid NoMethodError on nil REDIS_VERSION = "0.0.0" + # + # @return [:legacy] default digest algorithm :modern or :legacy + DIGEST_ALGORITHM = :legacy # # Returns a default configuration @@ -198,6 +200,7 @@ def self.default # rubocop:disable Metrics/MethodLength USE_LOCK_INFO, RAISE_ON_CONFIG_ERROR, REDIS_VERSION, + DIGEST_ALGORITHM, ) end @@ -304,6 +307,21 @@ def add_strategy(name, klass) self.strategies = new_strategies end + # + # Sets digest_algorithm to either :modern or :legacy + # + # @param [Symbol] value + # + # @return [Symbol] the new value + # + def digest_algorithm=(value) + unless [:modern, :legacy].include?(value) + raise ArgumentError, "Invalid digest algorithm: #{value} (should be :modern or :legacy)" + end + + super + end + # # The current version of redis # diff --git a/lib/sidekiq_unique_jobs/digests.rb b/lib/sidekiq_unique_jobs/digests.rb index efab093f9..75ee059ed 100644 --- a/lib/sidekiq_unique_jobs/digests.rb +++ b/lib/sidekiq_unique_jobs/digests.rb @@ -18,7 +18,7 @@ class Digests < Redis::SortedSet EMPTY_KEYS_SEGMENT = ["", "", "", ""].freeze def initialize(digests_key = DIGESTS) - super(digests_key) + super end # diff --git a/lib/sidekiq_unique_jobs/lock/while_executing.rb b/lib/sidekiq_unique_jobs/lock/while_executing.rb index 320061197..49961f7e4 100644 --- a/lib/sidekiq_unique_jobs/lock/while_executing.rb +++ b/lib/sidekiq_unique_jobs/lock/while_executing.rb @@ -23,7 +23,7 @@ class WhileExecuting < BaseLock # @param [Sidekiq::RedisConnection, ConnectionPool] redis_pool the redis connection # def initialize(item, callback, redis_pool = nil) - super(item, callback, redis_pool) + super append_unique_key_suffix end diff --git a/lib/sidekiq_unique_jobs/lock_digest.rb b/lib/sidekiq_unique_jobs/lock_digest.rb index c81ec76ea..2318b6606 100644 --- a/lib/sidekiq_unique_jobs/lock_digest.rb +++ b/lib/sidekiq_unique_jobs/lock_digest.rb @@ -51,7 +51,12 @@ 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.sort)) + digest = if SidekiqUniqueJobs.config.digest_algorithm == :legacy + OpenSSL::Digest::MD5.hexdigest(dump_json(digestable_hash.sort)) + else + OpenSSL::Digest.new("SHA3-256", dump_json(digestable_hash.sort)).hexdigest + end + "#{lock_prefix}:#{digest}" end diff --git a/lib/sidekiq_unique_jobs/on_conflict/replace.rb b/lib/sidekiq_unique_jobs/on_conflict/replace.rb index 2c413085c..810653596 100644 --- a/lib/sidekiq_unique_jobs/on_conflict/replace.rb +++ b/lib/sidekiq_unique_jobs/on_conflict/replace.rb @@ -21,7 +21,7 @@ class Replace < OnConflict::Strategy # @param [Hash] item sidekiq job hash # def initialize(item, redis_pool = nil) - super(item, redis_pool) + super @queue = item[QUEUE] @lock_digest = item[LOCK_DIGEST] end diff --git a/lib/sidekiq_unique_jobs/on_conflict/reschedule.rb b/lib/sidekiq_unique_jobs/on_conflict/reschedule.rb index 45f569c28..694b27cbe 100644 --- a/lib/sidekiq_unique_jobs/on_conflict/reschedule.rb +++ b/lib/sidekiq_unique_jobs/on_conflict/reschedule.rb @@ -13,7 +13,7 @@ class Reschedule < OnConflict::Strategy # @param [Hash] item sidekiq job hash def initialize(item, redis_pool = nil) - super(item, redis_pool) + super self.job_class = item[CLASS] end diff --git a/lib/sidekiq_unique_jobs/orphans/manager.rb b/lib/sidekiq_unique_jobs/orphans/manager.rb index 0cf0fa982..5774f7f23 100644 --- a/lib/sidekiq_unique_jobs/orphans/manager.rb +++ b/lib/sidekiq_unique_jobs/orphans/manager.rb @@ -32,7 +32,8 @@ module Manager # # @return [SidekiqUniqueJobs::TimerTask] the task that was started # - def start(test_task = nil) # rubocop:disable + # rubocop:disable + def start(test_task = nil) return if disabled? return if registered? diff --git a/lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb b/lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb index 76cc29947..d5a82ac8e 100644 --- a/lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb +++ b/lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb @@ -57,7 +57,7 @@ class RubyReaper < Reaper # @param [Redis] conn a connection to redis # def initialize(conn) - super(conn) + super @digests = SidekiqUniqueJobs::Digests.new @scheduled = Redis::SortedSet.new(SCHEDULE) @retried = Redis::SortedSet.new(RETRY) diff --git a/lib/sidekiq_unique_jobs/sidekiq_unique_ext.rb b/lib/sidekiq_unique_jobs/sidekiq_unique_ext.rb index b003592a8..9c8778f6d 100644 --- a/lib/sidekiq_unique_jobs/sidekiq_unique_ext.rb +++ b/lib/sidekiq_unique_jobs/sidekiq_unique_ext.rb @@ -60,7 +60,7 @@ module UniqueExtension # def delete(score, job_id) entry = find_job(job_id) - SidekiqUniqueJobs::Unlockable.delete!(entry.item) if super(score, job_id) + SidekiqUniqueJobs::Unlockable.delete!(entry.item) if super entry end end @@ -132,7 +132,7 @@ def clear # @param [String] value a sidekiq job hash # def delete_by_value(name, value) - SidekiqUniqueJobs::Unlockable.delete!(Sidekiq.load_json(value)) if super(name, value) + SidekiqUniqueJobs::Unlockable.delete!(Sidekiq.load_json(value)) if super end end diff --git a/lib/sidekiq_unique_jobs/testing.rb b/lib/sidekiq_unique_jobs/testing.rb index a195b5050..9e4d80d05 100644 --- a/lib/sidekiq_unique_jobs/testing.rb +++ b/lib/sidekiq_unique_jobs/testing.rb @@ -87,7 +87,7 @@ module Overrides def sidekiq_options(options = {}) SidekiqUniqueJobs.validate_worker!(options) if SidekiqUniqueJobs.config.raise_on_config_error - super(options) + super end # diff --git a/spec/performance/lock_digest_spec.rb b/spec/performance/lock_digest_spec.rb index d6e681575..7160f5c49 100644 --- a/spec/performance/lock_digest_spec.rb +++ b/spec/performance/lock_digest_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# rubocop:disable RSpec/SpecFilePathFormat, RSpec/FilePath +# rubocop:disable RSpec/SpecFilePathFormat RSpec.describe SidekiqUniqueJobs::LockDigest, :perf do let(:lock_digest) { described_class.new(item) } let(:job_class) { UntilExecutedJob } @@ -51,4 +51,4 @@ end end end -# rubocop:enable RSpec/SpecFilePathFormat, RSpec/FilePath +# rubocop:enable RSpec/SpecFilePathFormat diff --git a/spec/performance/locksmith_spec.rb b/spec/performance/locksmith_spec.rb index 792d7cf4d..772231905 100644 --- a/spec/performance/locksmith_spec.rb +++ b/spec/performance/locksmith_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# rubocop:disable RSpec/SpecFilePathFormat, RSpec/FilePath +# rubocop:disable RSpec/SpecFilePathFormat RSpec.describe SidekiqUniqueJobs::Locksmith, :perf do let(:locksmith_one) { described_class.new(item_one) } let(:locksmith_two) { described_class.new(item_two) } @@ -46,4 +46,4 @@ expect { locksmith_one.unlock }.to perform_under(1).ms end end -# rubocop:enable RSpec/SpecFilePathFormat, RSpec/FilePath +# rubocop:enable RSpec/SpecFilePathFormat diff --git a/spec/sidekiq_unique_jobs/cli_spec.rb b/spec/sidekiq_unique_jobs/cli_spec.rb index 7c983b771..3a1313867 100644 --- a/spec/sidekiq_unique_jobs/cli_spec.rb +++ b/spec/sidekiq_unique_jobs/cli_spec.rb @@ -37,9 +37,9 @@ jobs del PATTERN Options: - -d, [--dry-run], [--no-dry-run] # set to false to perform deletion - -c, [--count=N] # The max number of digests to return - # Default: 1000 + -d, [--dry-run], [--no-dry-run], [--skip-dry-run] # set to false to perform deletion + -c, [--count=N] # The max number of digests to return + # Default: 1000 deletes unique digests from redis by pattern HEADER diff --git a/spec/sidekiq_unique_jobs/configuration_spec.rb b/spec/sidekiq_unique_jobs/configuration_spec.rb index 9434fb060..89351fbeb 100644 --- a/spec/sidekiq_unique_jobs/configuration_spec.rb +++ b/spec/sidekiq_unique_jobs/configuration_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# rubocop:disable RSpec/SpecFilePathFormat, RSpec/FilePath +# rubocop:disable RSpec/SpecFilePathFormat RSpec.describe SidekiqUniqueJobs do describe "define custom lock strategies" do subject(:middleware_call) do @@ -65,4 +65,4 @@ def lock end end end -# rubocop:enable RSpec/SpecFilePathFormat, RSpec/FilePath +# rubocop:enable RSpec/SpecFilePathFormat diff --git a/spec/sidekiq_unique_jobs/lock_digest_spec.rb b/spec/sidekiq_unique_jobs/lock_digest_spec.rb index 1187874d6..2e50cf8c5 100644 --- a/spec/sidekiq_unique_jobs/lock_digest_spec.rb +++ b/spec/sidekiq_unique_jobs/lock_digest_spec.rb @@ -35,7 +35,7 @@ let(:digest_two) { described_class.new(another_item) } context "with the same unique args" do - let(:another_item) { item } + let(:another_item) { item.dup } it "equals to lock_digest for that item" do expect(lock_digest).to eq(digest_two.lock_digest) @@ -52,18 +52,40 @@ end end - context "when digest is a proc" do - let(:job_class) { MyUniqueJobWithFilterProc } - let(:args) { [1, 2, { "type" => "it" }] } + context "when digest_algorithm is :legacy" do + context "when digest is a proc" do + let(:job_class) { MyUniqueJobWithFilterProc } + let(:args) { [1, 2, { "type" => "it" }] } - it_behaves_like "unique digest" + it_behaves_like "unique digest" + end + + context "when unique_args is a symbol" do + let(:job_class) { MyUniqueJobWithFilterMethod } + let(:args) { [1, 2, { "type" => "it" }] } + + it_behaves_like "unique digest" + end end - context "when unique_args is a symbol" do - let(:job_class) { MyUniqueJobWithFilterMethod } - let(:args) { [1, 2, { "type" => "it" }] } + context "when digest_algorithm is :modern" do + around do |example| + SidekiqUniqueJobs.use_config(digest_algorithm: :modern, &example) + end + + context "when digest is a proc" do + let(:job_class) { MyUniqueJobWithFilterProc } + let(:args) { [1, 2, { "type" => "it" }] } + + it_behaves_like "unique digest" + end - it_behaves_like "unique digest" + context "when unique_args is a symbol" do + let(:job_class) { MyUniqueJobWithFilterMethod } + let(:args) { [1, 2, { "type" => "it" }] } + + it_behaves_like "unique digest" + end end end diff --git a/spec/sidekiq_unique_jobs/middleware/server/until_and_while_executing_spec.rb b/spec/sidekiq_unique_jobs/middleware/server/until_and_while_executing_spec.rb index 57ed68250..499f8b285 100644 --- a/spec/sidekiq_unique_jobs/middleware/server/until_and_while_executing_spec.rb +++ b/spec/sidekiq_unique_jobs/middleware/server/until_and_while_executing_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# rubocop:disable RSpec/SpecFilePathFormat, RSpec/FilePath, RSpec/DescribeMethod +# rubocop:disable RSpec/SpecFilePathFormat, RSpec/DescribeMethod RSpec.describe SidekiqUniqueJobs::Middleware::Server, "lock: :until_and_while_executing" do let(:server) { described_class.new } @@ -46,4 +46,4 @@ end end end -# rubocop:enable RSpec/SpecFilePathFormat, RSpec/FilePath, RSpec/DescribeMethod +# rubocop:enable RSpec/SpecFilePathFormat, RSpec/DescribeMethod diff --git a/spec/sidekiq_unique_jobs/script/scripts_spec.rb b/spec/sidekiq_unique_jobs/script/scripts_spec.rb index f9c679eab..a3c8a096d 100644 --- a/spec/sidekiq_unique_jobs/script/scripts_spec.rb +++ b/spec/sidekiq_unique_jobs/script/scripts_spec.rb @@ -59,6 +59,6 @@ describe "#kill" do subject(:kill) { scripts.kill(redis) } - specify { expect { kill }.to raise_error(RedisClient::CommandError, "NOTBUSY No scripts in execution right now.") } + specify { expect { kill }.to raise_error(RedisClient::CommandError, /NOTBUSY/) } end end diff --git a/spec/workers/until_and_while_executing_reject_job_spec.rb b/spec/workers/until_and_while_executing_reject_job_spec.rb index 3f9f2b1cc..29529dfe7 100644 --- a/spec/workers/until_and_while_executing_reject_job_spec.rb +++ b/spec/workers/until_and_while_executing_reject_job_spec.rb @@ -4,10 +4,10 @@ it_behaves_like "sidekiq with options" do let(:options) do { + "lock" => :until_and_while_executing, + "on_conflict" => { "client" => :reject, "server" => :reject }, "queue" => :working, "retry" => true, - "lock" => :until_and_while_executing, - "on_conflict" => { client: :reject, server: :reject }, } end end diff --git a/spec/workers/until_and_while_executing_replace_job_spec.rb b/spec/workers/until_and_while_executing_replace_job_spec.rb index 13d889bc6..095521df2 100644 --- a/spec/workers/until_and_while_executing_replace_job_spec.rb +++ b/spec/workers/until_and_while_executing_replace_job_spec.rb @@ -7,7 +7,7 @@ "queue" => :working, "retry" => true, "lock" => :until_and_while_executing, - "on_conflict" => { client: :replace, server: :reschedule }, + "on_conflict" => { "client" => :replace, "server" => :reschedule }, } end end