diff --git a/.codeclimate.yml b/.codeclimate.yml index 3e810964b..7f6a9ab4e 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -1,36 +1,7 @@ --- -engines: - csslint: - enabled: true - duplication: - enabled: true - config: - languages: - - ruby - - javascript - - python - - php - eslint: - enabled: true - fixme: - enabled: true - rubocop: - enabled: true -ratings: - paths: - - "**.css" - - "**.inc" - - "**.js" - - "**.jsx" - - "**.module" - - "**.php" - - "**.py" - - "**.rb" -exclude_paths: - - spec/ engines: bundler-audit: - enabled: true + enabled: false fixme: enabled: true duplication: @@ -47,7 +18,7 @@ ratings: - "**.rb" exclude_paths: - Gemfile - - *.gemspec + - "*.gemspec" - Appraisals - spec/**/*.rb - gemfiles/**/* diff --git a/.editorconfig b/.editorconfig index 65d44abc7..aaeaf0ab6 100644 --- a/.editorconfig +++ b/.editorconfig @@ -10,5 +10,5 @@ insert_final_newline = true indent_style = space indent_size = 2 -[*.md] -trim_trailing_whitespace = true +[*.{md,slim,haml}] +trim_trailing_whitespace = false diff --git a/.rubocop.yml b/.rubocop.yml index bca4fb4e8..c154faa64 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -22,7 +22,7 @@ Metrics/CyclomaticComplexity: Max: 7 Metrics/LineLength: - Max: 108 + Max: 120 Metrics/MethodLength: Max: 13 diff --git a/.travis.yml b/.travis.yml index c280e8818..c7315d987 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,10 @@ sudo: false language: ruby cache: bundler -before_install: - - rvm get head - - gem update --system - - gem install bundler +# before_install: +# - rvm get head +# - gem update --system +# - gem install bundler services: - redis-server script: diff --git a/CHANGELOG.md b/CHANGELOG.md index efa6ea934..d3eed6652 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## v5.0.1 + +- Added a command line util for cleaning out expired keys +- Use `SidekiqUniqueJobs.logger` instead of spreading out `Sidekiq.logger` everywhere. + ## v5.0.0 - Only support Sidekiq >= 4 diff --git a/lib/sidekiq-unique-jobs.rb b/lib/sidekiq-unique-jobs.rb index f3df84619..d4d9e438f 100644 --- a/lib/sidekiq-unique-jobs.rb +++ b/lib/sidekiq-unique-jobs.rb @@ -26,10 +26,14 @@ def config default_queue_lock_expiration: 30 * 60, default_run_lock_expiration: 60, default_lock: :while_executing, - redis_test_mode: :redis # :mock + redis_test_mode: :redis, # :mock ) end + def logger + Sidekiq.logger + end + def default_lock config.default_lock end diff --git a/lib/sidekiq/simulator.rb b/lib/sidekiq/simulator.rb index f16878db7..7daba422a 100644 --- a/lib/sidekiq/simulator.rb +++ b/lib/sidekiq/simulator.rb @@ -3,6 +3,9 @@ module Sidekiq class Simulator + extend Forwardable + def_delegator SidekiqUniqueJobs, :logger + attr_reader :queues, :launcher def self.process_queue(queue) @@ -67,9 +70,5 @@ def sidekiq_options(queues = []) verbose: false, logfile: './tmp/sidekiq.log' } end - - def logger - @logger ||= Sidekiq.logger - end end end diff --git a/lib/sidekiq_unique_jobs/cli.rb b/lib/sidekiq_unique_jobs/cli.rb index d6a2b8681..3104fb632 100644 --- a/lib/sidekiq_unique_jobs/cli.rb +++ b/lib/sidekiq_unique_jobs/cli.rb @@ -2,30 +2,52 @@ module SidekiqUniqueJobs class Cli < Thor + # def initialize(argv, stdin = STDIN, stdout = STDOUT, stderr = STDERR, kernel = Kernel) + # @argv, @stdin, @stdout, @stderr, @kernel = argv, stdin, stdout, stderr, kernel + # end + + def self.banner(command, _namespace = nil, _subcommand = false) + "jobs #{@package_name} #{command.usage}" + end + desc 'keys PATTERN', 'list all unique keys and their expiry time' option :count, aliases: :c, type: :numeric, default: 1000, desc: 'The max number of keys to return' def keys(pattern) - Util.keys(pattern, options[:count]) + keys = Util.keys(pattern, options[:count]) + say "Found #{keys.size} keys matching '#{pattern}':" + print_in_columns(keys.sort) if keys.any? end desc 'del PATTERN', 'deletes unique keys from redis by pattern' option :dry_run, aliases: :d, type: :boolean, desc: 'set to false to perform deletion' option :count, aliases: :c, type: :numeric, default: 1000, desc: 'The max number of keys to return' def del(pattern) - Util.del(pattern, options[:count], options[:dry_run]) + deleted_count = Util.del(pattern, options[:count], options[:dry_run]) + say "Deleted #{deleted_count} keys matching '#{pattern}'" + end + + desc 'expire', 'removes all expired unique keys from the hash in redis' + def expire + expired = Util.expire + say "Removed #{expired.values.size} left overs from redis." + print_in_columns(expired.values) end desc 'console', 'drop into a console with easy access to helper methods' def console - puts "Use `keys '*', 1000 to display the first 1000 unique keys matching '*'" - puts "Use `del '*', 1000, true (default) to see how many keys would be deleted for the pattern '*'" - puts "Use `del '*', 1000, false to delete the first 1000 keys matching '*'" + say "Use `keys '*', 1000 to display the first 1000 unique keys matching '*'" + say "Use `del '*', 1000, true (default) to see how many keys would be deleted for the pattern '*'" + say "Use `del '*', 1000, false to delete the first 1000 keys matching '*'" Object.include SidekiqUniqueJobs::Util console_class.start end private + def logger + SidekiqUniqueJobs.logger + end + def console_class require 'pry' Pry diff --git a/lib/sidekiq_unique_jobs/lock/until_executed.rb b/lib/sidekiq_unique_jobs/lock/until_executed.rb index b59f3b53f..dcdae3251 100644 --- a/lib/sidekiq_unique_jobs/lock/until_executed.rb +++ b/lib/sidekiq_unique_jobs/lock/until_executed.rb @@ -35,25 +35,17 @@ def unlock(scope) unlock_by_key(unique_key, item[JID_KEY], redis_pool) end - # rubocop:disable MethodLength def lock(scope) if scope.to_sym != :client raise ArgumentError, "#{scope} middleware can't #{__method__} #{unique_key}" end - result = Scripts.call(:acquire_lock, redis_pool, - keys: [unique_key], - argv: [item[JID_KEY], max_lock_time]) - case result - when 1 - logger.debug { "successfully locked #{unique_key} for #{max_lock_time} seconds" } - true - when 0 - logger.debug { "failed to acquire lock for #{unique_key}" } - false - else - raise "#{__method__} returned an unexpected value (#{result})" - end + Scripts::AcquireLock.execute( + redis_pool, + unique_key, + item[JID_KEY], + max_lock_time + ) end # rubocop:enable MethodLength diff --git a/lib/sidekiq_unique_jobs/scripts.rb b/lib/sidekiq_unique_jobs/scripts.rb index b8a72aa7f..9c470c30d 100644 --- a/lib/sidekiq_unique_jobs/scripts.rb +++ b/lib/sidekiq_unique_jobs/scripts.rb @@ -1,9 +1,15 @@ require 'pathname' require 'digest/sha1' require 'concurrent/map' +require 'sidekiq_unique_jobs/scripts/acquire_lock' +require 'sidekiq_unique_jobs/scripts/release_lock' module SidekiqUniqueJobs - ScriptError = Class.new(StandardError) + ScriptError = Class.new(StandardError) + UniqueKeyMissing = Class.new(ArgumentError) + JidMissing = Class.new(ArgumentError) + MaxLockTimeMissing = Class.new(ArgumentError) + UnexpectedValue = Class.new(StandardError) module Scripts LUA_PATHNAME ||= Pathname.new(__FILE__).dirname.join('../../redis').freeze @@ -14,11 +20,7 @@ module Scripts module_function extend SingleForwardable - def_delegator :SidekiqUniqueJobs, :connection - - def logger - Sidekiq.logger - end + def_delegators :SidekiqUniqueJobs, :connection, :logger def call(file_name, redis_pool, options = {}) # rubocop:disable MethodLength connection(redis_pool) do |redis| diff --git a/lib/sidekiq_unique_jobs/scripts/acquire_lock.rb b/lib/sidekiq_unique_jobs/scripts/acquire_lock.rb new file mode 100644 index 000000000..bdcfeba68 --- /dev/null +++ b/lib/sidekiq_unique_jobs/scripts/acquire_lock.rb @@ -0,0 +1,45 @@ +module SidekiqUniqueJobs + module Scripts + class AcquireLock + extend Forwardable + def_delegator SidekiqUniqueJobs, :logger + + def self.execute(redis_pool, unique_key, jid, max_lock_time) + new(redis_pool, unique_key, jid, max_lock_time).execute + end + + attr_reader :redis_pool, :unique_key, :jid, :max_lock_time + + def initialize(_redis_pool, unique_key, jid, max_lock_time) + raise UniqueKeyMissing, 'unique_key is required' if unique_key.nil? + raise JidMissing, 'jid is required' if jid.nil? + raise MaxLockTimeMissing, 'max_lock_time is required' if max_lock_time.nil? + + @unique_key = unique_key + @jid = jid + @max_lock_time = max_lock_time + end + + def execute + result = Scripts.call(:acquire_lock, redis_pool, + keys: [unique_key], + argv: [jid, max_lock_time]) + + handle_result(result) + end + + def handle_result(result) + case result + when 1 + logger.debug { "successfully acquired lock #{unique_key} for #{max_lock_time} seconds" } + true + when 0 + logger.debug { "failed to acquire lock for #{unique_key}" } + false + else + raise UnexpectedValue, "failed to acquire lock : unexpected return value (#{result})" + end + end + end + end +end diff --git a/lib/sidekiq_unique_jobs/scripts/release_lock.rb b/lib/sidekiq_unique_jobs/scripts/release_lock.rb new file mode 100644 index 000000000..4005575f4 --- /dev/null +++ b/lib/sidekiq_unique_jobs/scripts/release_lock.rb @@ -0,0 +1,47 @@ +module SidekiqUniqueJobs + module Scripts + class ReleaseLock + extend Forwardable + def_delegator SidekiqUniqueJobs, :logger + + def self.execute(redis_pool, unique_key, jid) + new(redis_pool, unique_key, jid).execute + end + + attr_reader :redis_pool, :unique_key, :jid + + def initialize(redis_pool, unique_key, jid) + raise UniqueKeyMissing, 'unique_key is required' if unique_key.nil? + raise JidMissing, 'jid is required' if jid.nil? + + @redis_pool = redis_pool + @unique_key = unique_key + @jid = jid + end + + def execute + result = Scripts.call(:release_lock, redis_pool, + keys: [unique_key], + argv: [jid]) + + handle_result(result) + end + + def handle_result(result) + case result + when 1 + logger.debug { "successfully unlocked #{unique_key}" } + true + when 0 + logger.debug { "expiring lock #{unique_key} is not owned by #{jid}" } + false + when -1 + logger.debug { "#{unique_key} is not a known key" } + false + else + raise UnexpectedValue, "failed to release lock : unexpected return value (#{result})" + end + end + end + end +end diff --git a/lib/sidekiq_unique_jobs/testing.rb b/lib/sidekiq_unique_jobs/testing.rb index 4c72214dc..6cf986f60 100644 --- a/lib/sidekiq_unique_jobs/testing.rb +++ b/lib/sidekiq_unique_jobs/testing.rb @@ -25,7 +25,7 @@ class << self module Testing def call_ext(file_name, redis_pool, options = {}) - if SidekiqUniqueJobs.config.redis_test_mode == :mock + if SidekiqUniqueJobs.mocked? SidekiqUniqueJobs::ScriptMock.call(file_name, redis_pool, options) else call_orig(file_name, redis_pool, options) diff --git a/lib/sidekiq_unique_jobs/unique_args.rb b/lib/sidekiq_unique_jobs/unique_args.rb index fa9c7795f..3cf532780 100644 --- a/lib/sidekiq_unique_jobs/unique_args.rb +++ b/lib/sidekiq_unique_jobs/unique_args.rb @@ -19,10 +19,10 @@ def self.digest(item) def initialize(job) Sidekiq::Logging.with_context(CLASS_NAME) do @item = job - @worker_class ||= worker_class_constantize(@item[CLASS_KEY]) + @worker_class ||= worker_class_constantize(@item[CLASS_KEY]) @item[UNIQUE_PREFIX_KEY] ||= unique_prefix - @item[UNIQUE_ARGS_KEY] = unique_args(@item[ARGS_KEY]) # SIC! Calculate unique_args unconditionally - @item[UNIQUE_DIGEST_KEY] = unique_digest + @item[UNIQUE_ARGS_KEY] = unique_args(@item[ARGS_KEY]) + @item[UNIQUE_DIGEST_KEY] = unique_digest end end diff --git a/lib/sidekiq_unique_jobs/unlockable.rb b/lib/sidekiq_unique_jobs/unlockable.rb index cf3a6839b..a4ae0883f 100644 --- a/lib/sidekiq_unique_jobs/unlockable.rb +++ b/lib/sidekiq_unique_jobs/unlockable.rb @@ -7,26 +7,12 @@ def unlock(item) end def unlock_by_key(unique_key, jid, redis_pool = nil) - result = Scripts.call(:release_lock, redis_pool, keys: [unique_key], argv: [jid]) - after_unlock(result, __method__, unique_key, jid) + return false unless Scripts::ReleaseLock.execute(redis_pool, unique_key, jid) + after_unlock(jid) end - def after_unlock(result, calling_method, unique_key, jid) # rubocop:disable Metrics/MethodLength + def after_unlock(jid) ensure_job_id_removed(jid) - - case result - when 1 - logger.debug { "successfully unlocked #{unique_key}" } - true - when 0 - logger.debug { "expiring lock #{unique_key} is not owned by #{jid}" } - false - when -1 - logger.debug { "#{unique_key} is not a known key" } - false - else - raise "#{calling_method} returned an unexpected value (#{result})" - end end def ensure_job_id_removed(jid) @@ -34,7 +20,7 @@ def ensure_job_id_removed(jid) end def logger - Sidekiq.logger + SidekiqUniqueJobs.logger end end end diff --git a/lib/sidekiq_unique_jobs/util.rb b/lib/sidekiq_unique_jobs/util.rb index 83dcaac83..fe87b1599 100644 --- a/lib/sidekiq_unique_jobs/util.rb +++ b/lib/sidekiq_unique_jobs/util.rb @@ -18,7 +18,7 @@ def unique_key(jid) end def del(pattern = SCAN_PATTERN, count = 0, dry_run = true) - raise 'Please provide a number of keys to delete greater than zero' if count.zero? + raise ArgumentError, 'Please provide a number of keys to delete greater than zero' if count.zero? logger.debug { "Deleting keys by: #{pattern}" } keys, time = timed { keys(pattern, count) } logger.debug { "#{keys.size} matching keys found in #{time} sec." } @@ -32,6 +32,24 @@ def del(pattern = SCAN_PATTERN, count = 0, dry_run = true) keys.size end + def unique_hash + connection do |conn| + conn.hgetall(SidekiqUniqueJobs::HASH_KEY) + end + end + + def expire + removed_keys = {} + connection do |conn| + conn.hgetall(SidekiqUniqueJobs::HASH_KEY).each do |jid, unique_key| + next if conn.get(unique_key) + conn.hdel(SidekiqUniqueJobs::HASH_KEY, jid) + removed_keys[jid] = unique_key + end + end + removed_keys + end + def keys_by_scan(pattern, count) connection { |conn| conn.scan_each(match: prefix(pattern), count: count).to_a } end @@ -72,6 +90,7 @@ def prefix_keys(keys) def prefix(key) return key if unique_prefix.nil? + return key if key.start_with?("#{unique_prefix}:") "#{unique_prefix}:#{key}" end @@ -92,7 +111,7 @@ def redis_keys_method end def logger - Sidekiq.logger + SidekiqUniqueJobs.logger end end end diff --git a/rails_example/.env b/rails_example/.env index 918d09847..3ae005a73 100644 --- a/rails_example/.env +++ b/rails_example/.env @@ -3,7 +3,7 @@ APP_DOMAIN=localhost APP_WEB_URL=http://localhost:3000 APP_CABLE_URL=ws://localhost:28080 WEB_CONSOLE_WHITELISTED_IPS=127.0.0.1 ::1 127.0.0.0/8 ::1 -DB_HOST=postgres +DB_HOST=localhost DB_PORT=5432 DB_USERNAME=mhenrixon DB_PASSWORD= diff --git a/rails_example/Gemfile b/rails_example/Gemfile index a7153857a..f7673f4dd 100644 --- a/rails_example/Gemfile +++ b/rails_example/Gemfile @@ -25,6 +25,7 @@ gem 'sidekiq-unique-jobs', path: '../' gem 'sinatra', github: 'sinatra/sinatra' group :development do + gem 'foreman' gem 'web-console' end diff --git a/rails_example/Procfile b/rails_example/Procfile index b28132370..e36154a63 100644 --- a/rails_example/Procfile +++ b/rails_example/Procfile @@ -1,2 +1,2 @@ web: bundle exec rails server -p $PORT -worker: bundle exec sidekiq +worker: bundle exec sidekiq -C config/sidekiq.yml diff --git a/rails_example/app/workers/simple_worker.rb b/rails_example/app/workers/simple_worker.rb index 4416eecf2..ffb03947e 100644 --- a/rails_example/app/workers/simple_worker.rb +++ b/rails_example/app/workers/simple_worker.rb @@ -8,7 +8,7 @@ class SimpleWorker def perform(some_args) Sidekiq::Logging.with_context(self.class.name) do - Sidekiq.logger.debug { "#{__method__}(#{some_args})" } + SidekiqUniqueJobs.logger.debug { "#{__method__}(#{some_args})" } end sleep 1 end diff --git a/rails_example/app/workers/slow_until_executing_worker.rb b/rails_example/app/workers/slow_until_executing_worker.rb index 390cb1860..dae20bc34 100644 --- a/rails_example/app/workers/slow_until_executing_worker.rb +++ b/rails_example/app/workers/slow_until_executing_worker.rb @@ -8,7 +8,7 @@ class SlowUntilExecutingWorker def perform(some_args) Sidekiq::Logging.with_context(self.class.name) do - Sidekiq.logger.debug { "#{__method__}(#{some_args})" } + SidekiqUniqueJobs.logger.debug { "#{__method__}(#{some_args})" } end sleep 15 end diff --git a/rails_example/app/workers/spawn_simple_worker.rb b/rails_example/app/workers/spawn_simple_worker.rb index 44368c9cb..705286635 100644 --- a/rails_example/app/workers/spawn_simple_worker.rb +++ b/rails_example/app/workers/spawn_simple_worker.rb @@ -3,7 +3,7 @@ class SpawnSimpleWorker def perform(spawn_arg) Sidekiq::Logging.with_context(self.class.name) do - Sidekiq.logger.debug { "#{__method__}(#{spawn_arg})" } + logger.debug { "#{__method__}(#{spawn_arg})" } end SimpleWorker.perform_async spawn_arg end diff --git a/rails_example/app/workers/while_executing_worker.rb b/rails_example/app/workers/while_executing_worker.rb new file mode 100644 index 000000000..965455d8d --- /dev/null +++ b/rails_example/app/workers/while_executing_worker.rb @@ -0,0 +1,12 @@ +class WhileExecutingWorker + include Sidekiq::Worker + + sidekiq_options unique: :while_executing + + def perform(_one, _two) + Sidekiq::Logging.with_context(self.class.name) do + logger.debug { "#{__method__}(#{some_args})" } + end + sleep 10 + end +end diff --git a/rails_example/app/workers/without_argument_worker.rb b/rails_example/app/workers/without_argument_worker.rb index afbb7639b..50386abf0 100644 --- a/rails_example/app/workers/without_argument_worker.rb +++ b/rails_example/app/workers/without_argument_worker.rb @@ -5,7 +5,7 @@ class WithoutArgumentWorker def perform Sidekiq::Logging.with_context(self.class.name) do - Sidekiq.logger.debug { __method__.to_s } + logger.debug { __method__.to_s } end sleep 20 end diff --git a/sidekiq-unique-jobs.gemspec b/sidekiq-unique-jobs.gemspec index a54351061..13a68712f 100644 --- a/sidekiq-unique-jobs.gemspec +++ b/sidekiq-unique-jobs.gemspec @@ -23,5 +23,6 @@ Gem::Specification.new do |gem| gem.add_development_dependency 'timecop' gem.add_development_dependency 'yard' gem.add_development_dependency 'gem-release' + gem.add_development_dependency 'aruba' gem.add_development_dependency 'codeclimate-test-reporter', '~> 1.0.0' end diff --git a/spec/jobs/another_unique_job.rb b/spec/jobs/another_unique_job.rb index 2450c8347..9a4d926d4 100644 --- a/spec/jobs/another_unique_job.rb +++ b/spec/jobs/another_unique_job.rb @@ -4,7 +4,7 @@ class AnotherUniqueJob unique: :until_executed sidekiq_retries_exhausted do |msg| - Sidekiq.logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" + logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" end def perform(*) diff --git a/spec/jobs/my_job.rb b/spec/jobs/my_job.rb index 4f1a54405..ef75138ca 100644 --- a/spec/jobs/my_job.rb +++ b/spec/jobs/my_job.rb @@ -3,7 +3,7 @@ class MyJob sidekiq_options queue: :working, retry: 1, backtrace: 10, unique: false sidekiq_retries_exhausted do |msg| - Sidekiq.logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" + logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" end def perform(*) diff --git a/spec/jobs/unique_job_with_filter_method.rb b/spec/jobs/unique_job_with_filter_method.rb index ac410a45a..ce4729e8f 100644 --- a/spec/jobs/unique_job_with_filter_method.rb +++ b/spec/jobs/unique_job_with_filter_method.rb @@ -4,7 +4,7 @@ class UniqueJobWithFilterMethod unique: :while_executing, unique_args: :filtered_args sidekiq_retries_exhausted do |msg| - Sidekiq.logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" + logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" end def perform(*) diff --git a/spec/jobs/unique_on_all_queues_job.rb b/spec/jobs/unique_on_all_queues_job.rb index 7f7a5cd61..bfc948041 100644 --- a/spec/jobs/unique_on_all_queues_job.rb +++ b/spec/jobs/unique_on_all_queues_job.rb @@ -4,7 +4,7 @@ class UniqueOnAllQueuesJob unique: :until_executed, unique_on_all_queues: true sidekiq_retries_exhausted do |msg| - Sidekiq.logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" + logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" end def perform(*) diff --git a/spec/jobs/until_executed_job.rb b/spec/jobs/until_executed_job.rb index ed4467fca..27e4cd8b2 100644 --- a/spec/jobs/until_executed_job.rb +++ b/spec/jobs/until_executed_job.rb @@ -4,7 +4,7 @@ class UntilExecutedJob sidekiq_options unique: :until_executed sidekiq_retries_exhausted do |msg| - Sidekiq.logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" + logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" end def perform(*) diff --git a/spec/jobs/while_executing_job.rb b/spec/jobs/while_executing_job.rb index d1886845b..da2fe0682 100644 --- a/spec/jobs/while_executing_job.rb +++ b/spec/jobs/while_executing_job.rb @@ -3,7 +3,7 @@ class WhileExecutingJob sidekiq_options queue: :working, retry: 1, backtrace: 10, unique: :while_executing sidekiq_retries_exhausted do |msg| - Sidekiq.logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" + logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" end def perform(_) diff --git a/spec/lib/sidekiq_unique_jobs/cli_spec.rb b/spec/lib/sidekiq_unique_jobs/cli_spec.rb new file mode 100644 index 000000000..63130a79c --- /dev/null +++ b/spec/lib/sidekiq_unique_jobs/cli_spec.rb @@ -0,0 +1,183 @@ +require 'spec_helper' +require 'thor/runner' + +RSpec.describe SidekiqUniqueJobs::Cli, ruby_ver: '>= 2.4' do + describe '#help' do + let(:output) { capture(:stdout) { described_class.start(%w[help]) } } + let(:banner) do + <<~EOS.freeze + Commands: + jobs console # drop into a console with easy access to helper methods + jobs del PATTERN # deletes unique keys from redis by pattern + jobs expire # removes all expired unique keys from the hash in redis + jobs help [COMMAND] # Describe available commands or one specific command + jobs keys PATTERN # list all unique keys and their expiry time + EOS + end + + it 'displays help' do + expect(output).to include(banner) + end + + describe '#help del' do + let(:output) { capture(:stdout) { described_class.start(%w[help del]) } } + let(:banner) do + <<~EOS.freeze + Usage: + jobs del PATTERN + + Options: + d, [--dry-run], [--no-dry-run] # set to false to perform deletion + c, [--count=N] # The max number of keys to return + # Default: 1000 + + deletes unique keys from redis by pattern + EOS + end + + it 'displays help about the `del` command' do + expect(output).to eq(banner) + end + end + + describe '#help expire' do + let(:output) { capture(:stdout) { described_class.start(%w[help expire]) } } + + let(:banner) do + <<~EOS.freeze + Usage: + jobs expire + + removes all expired unique keys from the hash in redis + EOS + end + + it 'displays help about the `expire` command' do + expect(output).to eq(banner) + end + end + + describe '#help keys' do + let(:output) { capture(:stdout) { described_class.start(%w[help keys]) } } + let(:banner) do + <<~EOS.freeze + Usage: + jobs keys PATTERN + + Options: + c, [--count=N] # The max number of keys to return + # Default: 1000 + + list all unique keys and their expiry time + EOS + end + + it 'displays help about the `key` command' do + expect(output).to eq(banner) + end + end + end + + let(:pattern) { '*' } + let(:max_lock_time) { 1 } + let(:unique_key) { 'uniquejobs:abcdefab' } + let(:jid) { 'abcdefab' } + + describe '.keys' do + let(:output) { capture(:stdout) { described_class.start(%w[keys * --count 1000]) } } + + context 'when no keys exist' do + let(:expected) { "Found 0 keys matching '#{pattern}':\n" } + specify { expect(output).to eq(expected) } + end + + context 'when a key exists' do + let(:keys) { ['defghayl', jid, 'poilkij'].sort } + before do + keys.each do |jid| + unique_key = "uniquejobs:#{jid}" + SidekiqUniqueJobs::Scripts::AcquireLock.execute(nil, unique_key, jid, 20) + expect(SidekiqUniqueJobs) + .to have_key(unique_key) + .for_seconds(20) + .with_value(jid) + end + end + + after { SidekiqUniqueJobs::Util.del('*', 1000, false) } + + let(:expected) do + <<~EOS + Found 3 keys matching '#{pattern}': + uniquejobs:abcdefab uniquejobs:defghayl uniquejobs:poilkij + EOS + end + specify do + expect(output).to eq(expected) + end + end + end + + describe '.del' do + let(:expected) do + <<~EOS + Deleted 1 keys matching '*' + EOS + end + + before do + SidekiqUniqueJobs::Scripts::AcquireLock.execute(nil, unique_key, jid, max_lock_time) + expect(SidekiqUniqueJobs) + .to have_key(unique_key) + .for_seconds(1) + .with_value(jid) + end + + specify do + output = capture(:stdout) { described_class.start(%w[del * --no-dry-run --count 1000]) } + expect(output).to eq(expected) + expect(SidekiqUniqueJobs).not_to have_key(unique_key) + end + end + + describe '.expire' do + before do + SidekiqUniqueJobs::Scripts::AcquireLock.execute(nil, unique_key, jid, max_lock_time) + expect(SidekiqUniqueJobs) + .to have_key(unique_key) + .for_seconds(1) + .with_value(jid) + end + let(:expected) do + <<~EOS + Removed 1 left overs from redis. + uniquejobs:abcdefab + EOS + end + + specify do + sleep 1 + output = capture(:stdout) { described_class.start(%w[expire]) } + expect(output).to eq(expected) + end + end + + describe '.console' do + let(:expected) do + <<~EOS + Use `keys '*', 1000 to display the first 1000 unique keys matching '*' + Use `del '*', 1000, true (default) to see how many keys would be deleted for the pattern '*' + Use `del '*', 1000, false to delete the first 1000 keys matching '*' + EOS + end + let(:output) { capture(:stdout) { described_class.start(%w[console]) } } + + specify do + expect(Object).to receive(:include).with(SidekiqUniqueJobs::Util).and_return(true) + console = double(:console) + allow_any_instance_of(SidekiqUniqueJobs::Cli).to receive(:console_class).and_return(console) + allow(console).to receive(:start) + expect(output).to eq(expected) + end + end +end diff --git a/spec/lib/sidekiq_unique_jobs/client/middleware_spec.rb b/spec/lib/sidekiq_unique_jobs/client/middleware_spec.rb index 07ece3d83..25dcec02e 100644 --- a/spec/lib/sidekiq_unique_jobs/client/middleware_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/client/middleware_spec.rb @@ -9,14 +9,6 @@ def digest_for(item) end describe 'with real redis' do - before do - if defined?(Sidekiq::Extensions) && Sidekiq::Extensions.respond_to?(:enable_delay!) - Sidekiq::Extensions.enable_delay! - end - Sidekiq.redis = REDIS - Sidekiq.redis(&:flushdb) - end - describe 'when a job is already scheduled' do it 'processes jobs properly' do Sidekiq::Testing.disable! do @@ -288,7 +280,7 @@ def do_it(_arg) end it 'does not log duplicate payload when config turned off' do - expect(Sidekiq.logger).to_not receive(:warn).with(/^payload is not unique/) + expect(SidekiqUniqueJobs.logger).to_not receive(:warn).with(/^payload is not unique/) UntilExecutedJob.sidekiq_options log_duplicate_payload: false diff --git a/spec/lib/sidekiq_unique_jobs/lock/until_and_while_executing_spec.rb b/spec/lib/sidekiq_unique_jobs/lock/until_and_while_executing_spec.rb index d942cfcf9..f2a3d9b4b 100644 --- a/spec/lib/sidekiq_unique_jobs/lock/until_and_while_executing_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/lock/until_and_while_executing_spec.rb @@ -15,8 +15,6 @@ describe '#execute' do before do - Sidekiq.redis(&:flushdb) - Sidekiq::Worker.clear_all subject.lock(:client) end let(:runtime_lock) { SidekiqUniqueJobs::Lock::WhileExecuting.new(item, nil) } diff --git a/spec/lib/sidekiq_unique_jobs/script_mock_spec.rb b/spec/lib/sidekiq_unique_jobs/script_mock_spec.rb index 41d2ad061..88765939c 100644 --- a/spec/lib/sidekiq_unique_jobs/script_mock_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/script_mock_spec.rb @@ -14,7 +14,6 @@ ANOTHER_JID ||= 'anotherjid'.freeze before do - Sidekiq.redis(&:flushdb) SidekiqUniqueJobs.configure do |config| config.redis_test_mode = :mock end @@ -36,7 +35,6 @@ SidekiqUniqueJobs.configure do |config| config.redis_test_mode = :redis end - Sidekiq.redis(&:flushdb) end subject { SidekiqUniqueJobs::Scripts } @@ -54,7 +52,7 @@ def unlock(key = UNIQUE_KEY, jid = JID) specify { expect(lock_for).to eq(1) } specify do expect(lock_for(1)).to eq(1) - expect(Redis) + expect(SidekiqUniqueJobs) .to have_key(UNIQUE_KEY) .for_seconds(1) .with_value('fuckit') diff --git a/spec/lib/sidekiq_unique_jobs/scripts/acquire_lock_spec.rb b/spec/lib/sidekiq_unique_jobs/scripts/acquire_lock_spec.rb new file mode 100644 index 000000000..1e17e1f01 --- /dev/null +++ b/spec/lib/sidekiq_unique_jobs/scripts/acquire_lock_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +RSpec.describe SidekiqUniqueJobs::Scripts::AcquireLock do + let(:redis_pool) { nil } + let(:jid) { 'abcdefab' } + let(:unique_key) { 'uniquejobs:123asdasd2134' } + let(:max_lock_time) { 1 } + + describe '.execute' do + subject { instance_double(described_class) } + + it 'delegates to instance' do + expect(described_class).to receive(:new) + .with(redis_pool, unique_key, jid, max_lock_time) + .and_return(subject) + expect(subject).to receive(:execute).and_return(true) + + described_class.execute(redis_pool, unique_key, jid, max_lock_time) + end + end + + describe '#execute' do + context 'when job is unique' do + def execute(myjid = jid, key = unique_key, max_lock = max_lock_time) + described_class.execute( + redis_pool, + key, + myjid, + max_lock + ) + end + + specify { expect(execute(jid, unique_key, max_lock_time)).to eq(true) } + specify do + expect(execute(jid, unique_key, max_lock_time)).to eq(true) + expect(SidekiqUniqueJobs) + .to have_key(unique_key) + .for_seconds(max_lock_time) + .with_value('abcdefab') + sleep 0.5 + expect(execute).to eq(true) + end + + context 'when a unique_key exists for another jid' do + before { expect(execute(jid, unique_key, 10)).to eq(true) } + specify { expect(execute('anotherjid', unique_key, 5)).to eq(false) } + end + end + end +end diff --git a/spec/lib/sidekiq_unique_jobs/scripts/release_lock_spec.rb b/spec/lib/sidekiq_unique_jobs/scripts/release_lock_spec.rb new file mode 100644 index 000000000..0fb8b499e --- /dev/null +++ b/spec/lib/sidekiq_unique_jobs/scripts/release_lock_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +RSpec.describe SidekiqUniqueJobs::Scripts::ReleaseLock do + let(:redis_pool) { nil } + let(:jid) { 'abcdefab' } + let(:unique_key) { 'uniquejobs:123asdasd2134' } + let(:max_lock_time) { 1 } + + describe '.execute' do + subject { instance_double(described_class) } + + it 'delegates to instance' do + expect(described_class).to receive(:new) + .with(redis_pool, unique_key, jid) + .and_return(subject) + expect(subject).to receive(:execute).and_return(true) + + described_class.execute(redis_pool, unique_key, jid) + end + end + + describe '#execute' do + context 'when exists' do + subject { described_class.execute(redis_pool, unique_key, jid) } + + before do + SidekiqUniqueJobs::Scripts::AcquireLock.execute(redis_pool, unique_key, jid, max_lock_time) + end + + specify do + expect(SidekiqUniqueJobs) + .to have_key(unique_key) + .for_seconds(max_lock_time) + .with_value(jid) + + expect(subject).to eq(true) + expect(SidekiqUniqueJobs).not_to have_key(unique_key) + end + end + end +end diff --git a/spec/lib/sidekiq_unique_jobs/scripts_spec.rb b/spec/lib/sidekiq_unique_jobs/scripts_spec.rb index 8760cca6d..9dd3e4bff 100644 --- a/spec/lib/sidekiq_unique_jobs/scripts_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/scripts_spec.rb @@ -6,10 +6,6 @@ ANOTHER_JID ||= 'anotherjid'.freeze context 'class methods' do - before do - Sidekiq.redis(&:flushdb) - Sidekiq::Worker.clear_all - end subject { SidekiqUniqueJobs::Scripts } it { is_expected.to respond_to(:call).with(3).arguments } @@ -34,12 +30,12 @@ def unlock(key = UNIQUE_KEY, jid = JID) context 'when job is unique' do specify { expect(lock_for).to eq(1) } specify do - expect(lock_for(0.5)).to eq(1) - expect(Redis) + expect(lock_for(2)).to eq(1) + expect(SidekiqUniqueJobs) .to have_key(UNIQUE_KEY) .for_seconds(1) - .with_value('fuckit') - sleep 0.5 + .with_value(JID) + sleep 2 expect(lock_for).to eq(1) end diff --git a/spec/lib/sidekiq_unique_jobs/server/middleware_spec.rb b/spec/lib/sidekiq_unique_jobs/server/middleware_spec.rb index 774e6d272..1fbc12a5c 100644 --- a/spec/lib/sidekiq_unique_jobs/server/middleware_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/server/middleware_spec.rb @@ -11,11 +11,6 @@ def digest_for(item) SidekiqUniqueJobs::UniqueArgs.digest(item) end - before do - Sidekiq.redis = REDIS - Sidekiq.redis(&:flushdb) - end - describe '#call' do context 'when unique is disabled' do it 'does not use locking' do diff --git a/spec/lib/sidekiq_unique_jobs/sidekiq_testing_enabled_spec.rb b/spec/lib/sidekiq_unique_jobs/sidekiq_testing_enabled_spec.rb index 96a7b0bef..a1ba9d491 100644 --- a/spec/lib/sidekiq_unique_jobs/sidekiq_testing_enabled_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/sidekiq_testing_enabled_spec.rb @@ -5,25 +5,6 @@ RSpec.describe 'When Sidekiq::Testing is enabled' do describe 'when set to :fake!', sidekiq: :fake do - before do - SidekiqUniqueJobs.configure do |config| - config.redis_test_mode = :redis - end - Sidekiq.redis = REDIS - Sidekiq.redis(&:flushdb) - Sidekiq::Worker.clear_all - if Sidekiq::Testing.respond_to?(:server_middleware) - Sidekiq::Testing.server_middleware do |chain| - chain.add SidekiqUniqueJobs::Server::Middleware - end - end - end - - after do - Sidekiq.redis(&:flushdb) - Sidekiq::Testing.server_middleware(&:clear) if Sidekiq::Testing.respond_to?(:server_middleware) - end - context 'with unique worker' do it 'does not push duplicate messages' do param = 'work' diff --git a/spec/lib/sidekiq_unique_jobs/sidekiq_unique_ext_spec.rb b/spec/lib/sidekiq_unique_jobs/sidekiq_unique_ext_spec.rb index 5da16fc5a..f81fed304 100644 --- a/spec/lib/sidekiq_unique_jobs/sidekiq_unique_ext_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/sidekiq_unique_ext_spec.rb @@ -6,11 +6,6 @@ require 'sidekiq_unique_jobs/sidekiq_unique_ext' RSpec.describe 'Sidekiq::Api' do - before do - Sidekiq.redis = REDIS - Sidekiq.redis(&:flushdb) - end - let(:item) do { 'class' => JustAWorker, 'queue' => 'testqueue', diff --git a/spec/lib/sidekiq_unique_jobs/unlockable_spec.rb b/spec/lib/sidekiq_unique_jobs/unlockable_spec.rb index 3093d86f0..858f9f564 100644 --- a/spec/lib/sidekiq_unique_jobs/unlockable_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/unlockable_spec.rb @@ -1,7 +1,6 @@ require 'spec_helper' RSpec.describe SidekiqUniqueJobs::Unlockable do - before { Sidekiq.redis(&:flushdb) } def item_with_digest SidekiqUniqueJobs::UniqueArgs.digest(item) item diff --git a/spec/lib/sidekiq_unique_jobs/util_spec.rb b/spec/lib/sidekiq_unique_jobs/util_spec.rb index a52764fb4..fcd091534 100644 --- a/spec/lib/sidekiq_unique_jobs/util_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/util_spec.rb @@ -1,47 +1,62 @@ require 'spec_helper' RSpec.describe SidekiqUniqueJobs::Util do - let(:keys) { %w[uniquejobs:keyz] } - - def set(key, value) - described_class.connection do |c| - c.set(key, value) - expect(c.keys('*')).to match_array([key]) - end + let(:job) do + { + 'class' => 'MyUniqueJob', + 'args' => [[1, 2]], + 'at' => 1_492_341_850.358196, + 'retry' => true, + 'queue' => 'customqueue', + 'unique' => :until_executed, + 'unique_expiration' => 7200, + 'retry_count' => 10, + 'jid' => jid, + 'created_at' => 1_492_341_790.358217, + } end + let(:unique_args) { SidekiqUniqueJobs::UniqueArgs.new(job) } + let(:unique_key) { unique_args.unique_digest } + let(:jid) { 'e3049b05b0bd9c809182bbe0' } - before(:each) do - Sidekiq.redis = REDIS - Sidekiq.redis(&:flushdb) + def acquire_lock + SidekiqUniqueJobs::Scripts::AcquireLock.execute(nil, unique_key, jid, 1) end describe '.keys' do end describe '.del' do - context 'given a key named "keyz" with value "valz"' do - before do - set('uniquejobs:keyz', 'valz') - end + before do + acquire_lock + end - it 'deletes the keys by pattern' do - expect(described_class.del('*', 100, false)).to eq(1) - end + it 'deletes the keys by pattern' do + expect(described_class.del(described_class::SCAN_PATTERN, 100, false)).to eq(1) + end - it 'deletes the keys by pattern' do - expect(described_class.del('keyz', 100, false)).to eq(1) - end + it 'deletes the keys by distinct key' do + expect(described_class.del(unique_key, 100, false)).to eq(1) end end - describe '.prefix' do - context 'when .unique_prefix is nil?' do - it 'does not prefix with unique_prefix' do - allow(SidekiqUniqueJobs.config).to receive(:unique_prefix).and_return(nil) - expect(described_class.prefix('key')).to eq('key') - end + describe '.expire' do + before do + acquire_lock end + it 'does some shit' do + sleep 1 + expected = { jid => unique_key } + expect(described_class.unique_hash).to match(expected) + removed_keys = described_class.expire + expect(removed_keys).to match(expected) + expect(described_class.unique_hash).not_to match(expected) + expect(described_class.keys('*')).not_to include(unique_key) + end + end + + describe '.prefix' do before do allow(SidekiqUniqueJobs.config).to receive(:unique_prefix).and_return('test-uniqueness') end @@ -49,5 +64,18 @@ def set(key, value) it 'returns a prefixed key' do expect(described_class.prefix('key')).to eq('test-uniqueness:key') end + + context 'when .unique_prefix is nil?' do + it 'does not prefix with unique_prefix' do + allow(SidekiqUniqueJobs.config).to receive(:unique_prefix).and_return(nil) + expect(described_class.prefix('key')).to eq('key') + end + end + + context 'when key is already prefixed' do + it 'does not add another prefix' do + expect(described_class.prefix('test-uniqueness:key')).to eq('test-uniqueness:key') + end + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a5369de31..29dc7f80d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -20,7 +20,7 @@ require 'sidekiq/simulator' Sidekiq::Testing.disable! -Sidekiq.logger.level = "Logger::#{ENV.fetch('LOGLEVEL') { 'error' }.upcase}".constantize +SidekiqUniqueJobs.logger.level = "Logger::#{ENV.fetch('LOGLEVEL') { 'error' }.upcase}".constantize require 'sidekiq/redis_connection' @@ -35,7 +35,8 @@ end Dir[File.join(File.dirname(__FILE__), 'support', '**', '*.rb')].each { |f| require f } -RSpec.configure do |config| + +RSpec.configure do |config| # rubocop:disable BlockLength config.expect_with :rspec do |expectations| expectations.include_chain_clauses_in_custom_matcher_descriptions = true end @@ -49,6 +50,43 @@ config.default_formatter = 'doc' if config.files_to_run.one? config.order = :random Kernel.srand config.seed + + config.before(:each) do + SidekiqUniqueJobs.configure do |unique_config| + unique_config.redis_test_mode = :redis + end + Sidekiq.redis = REDIS + Sidekiq.redis(&:flushdb) + Sidekiq::Worker.clear_all + Sidekiq::Queues.clear_all + + if Sidekiq::Testing.respond_to?(:server_middleware) + Sidekiq::Testing.server_middleware do |chain| + chain.add SidekiqUniqueJobs::Server::Middleware + end + end + enable_delay = defined?(Sidekiq::Extensions) && Sidekiq::Extensions.respond_to?(:enable_delay!) + Sidekiq::Extensions.enable_delay! if enable_delay + end + + config.after(:each) do + Sidekiq.redis(&:flushdb) + respond_to_middleware = defined?(Sidekiq::Testing) && Sidekiq::Testing.respond_to?(:server_middleware) + Sidekiq::Testing.server_middleware(&:clear) if respond_to_middleware + end end Dir[File.join(File.dirname(__FILE__), 'jobs', '**', '*.rb')].each { |f| require f } + +def capture(stream) + begin + stream = stream.to_s + eval "$#{stream} = StringIO.new" # rubocop:disable Security/Eval + yield + result = eval("$#{stream}").string # rubocop:disable Security/Eval + ensure + eval("$#{stream} = #{stream.upcase}") # rubocop:disable Security/Eval + end + + result +end diff --git a/spec/support/matchers/redis_matchers.rb b/spec/support/matchers/redis_matchers.rb index e2ec064f2..875ea0b51 100644 --- a/spec/support/matchers/redis_matchers.rb +++ b/spec/support/matchers/redis_matchers.rb @@ -2,18 +2,28 @@ RSpec::Matchers.define :have_key do |unique_key| Sidekiq.redis do |redis| - match do |_actual| - with_value && for_seconds + match do |_unique_jobs| + @value = redis.get(unique_key) + @ttl = redis.ttl(unique_key) + + @value && with_value && for_seconds end chain :with_value do |value = nil| - value.nil? || - redis.get(unique_key) == value + @expected_value = value + @expected_value && @value == @expected_value end chain :for_seconds do |ttl = nil| - ttl.nil? || - redis.ttl(unique_key) == ttl + @expected_ttl = ttl + @expected_ttl && @ttl == @expected_ttl + end + + failure_message do |_actual| + msg = "expected Redis to have key #{unique_key}" + msg << " with value #{@expected_value} was (#{@value})" if @expected_value + msg << " with value #{@expected_ttl} was (#{@ttl})" if @expected_ttl + msg end end end