Support both instance method and class method (mhenrixon#527)

* Support both instance method and class method

* Clarify documentation

* Reek

* Mandatory rubocop commit

* Adds missing coverage

* Fix broken code

* Mandatory rubocop commit

.reek.yml

@@ -78,6 +78,7 @@ detectors:
       - SidekiqUniqueJobs::OnConflict::Reject#deadset_kill?
       - SidekiqUniqueJobs::SidekiqWorkerMethods#worker_method_defined?
       - SidekiqUniqueJobs::Web::Helpers#redirect_to
+      - SidekiqUniqueJobs::SidekiqWorkerMethods#after_unlock_hook
   MissingSafeMethod:
     exclude:
       - Array

.rubocop.yml

@@ -4,11 +4,13 @@ inherit_gem:
 
 AllCops:
   TargetRubyVersion: 2.5
+  NewCops: enable
   Include:
     - "examples/**/*.rb"
   Exclude:
     - "**/*.erb"
     - "**/*.lua"
+    - "myapp"
     - "bin/bench"
 
 Layout/EndAlignment:
@@ -91,6 +93,9 @@ RSpec/InstanceVariable:
 RSpec/MultipleExpectations:
   Enabled: false
 
+RSpec/MultipleMemoizedHelpers:
+  Enabled: false
+
 RSpec/NestedGroups:
   Max: 4
   Enabled: true

Gemfile

@@ -37,7 +37,7 @@ platforms :mri do
 end
 
 if respond_to?(:install_if)
-  install_if -> { RUBY_PLATFORM =~ /darwin/ } do
+  install_if -> { RUBY_PLATFORM.include?("darwin") } do
     gem "fuubar"
     gem "pry"
     gem "rspec-nc"

README.md

@@ -562,17 +562,22 @@ If you need to perform any additional work after the lock has been released you
 **Exception 1:** UntilExecuting unlocks and uses callback before yielding.
 **Exception 2:** UntilExpired expires eventually, no after_unlock hook is called.
 
+**NOTE:** _It is also possible to write this code as a class method._
+
 ```ruby
 class UniqueJobWithFilterMethod
   include Sidekiq::Worker
   sidekiq_options lock: :while_executing,
 
+  def self.after_unlock
+   # block has yielded and lock is released
+  end
+
   def after_unlock
    # block has yielded and lock is released
   end
   ...
 end.
-```
 
 ### Logging
 

Rakefile

@@ -24,7 +24,7 @@ end
 
 def changed_files(pedantry)
   `git diff-tree --no-commit-id --name-only -r HEAD~#{pedantry} HEAD`
-    .split("\n").select { |f| f.match(/(\.rb\z)|Rakefile/) && File.exist?(f) && !f.match(/db/) }
+    .split("\n").select { |f| f.match(/(\.rb\z)|Rakefile/) && File.exist?(f) && f.include?(db) }
 end
 
 RuboCop::RakeTask.new(:rubocop) do |task|

bin/bundle

@@ -21,7 +21,7 @@ m = Module.new do
     ENV["BUNDLER_VERSION"]
   end
 
-  def cli_arg_version # rubocop:disable Metrics/CyclomaticComplexity
+  def cli_arg_version # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
     return unless invoked_as_script? # don't want to hijack other binstubs
     return unless "update".start_with?(ARGV.first || " ") # must be running `bundle update`
 

lib/sidekiq_unique_jobs.rb

@@ -26,7 +26,6 @@
 require "sidekiq_unique_jobs/exceptions"
 require "sidekiq_unique_jobs/script"
 require "sidekiq_unique_jobs/script/caller"
-require "sidekiq_unique_jobs/json"
 require "sidekiq_unique_jobs/normalizer"
 require "sidekiq_unique_jobs/job"
 require "sidekiq_unique_jobs/redis"

lib/sidekiq_unique_jobs/cli.rb

@@ -10,7 +10,7 @@ module SidekiqUniqueJobs
   #
   class Cli < Thor
     # :nodoc:
-    def self.banner(command, _namespace = nil, _subcommand = false)
+    def self.banner(command, _namespace = nil, _subcommand = false) # rubocop:disable Style/OptionalBooleanParameter
       "jobs #{@package_name} #{command.usage}" # rubocop:disable ThreadSafety/InstanceVariableInClassMethod
     end
 

lib/sidekiq_unique_jobs/lock_args.rb

@@ -97,7 +97,7 @@ def filter_by_symbol(args)
     end
 
     # The method to use for filtering unique arguments
-    def lock_args_method
+    def lock_args_method # rubocop:disable Metrics/CyclomaticComplexity
       @lock_args_method ||= worker_options[LOCK_ARGS] || worker_options[UNIQUE_ARGS]
       @lock_args_method ||= :lock_args if worker_method_defined?(:lock_args)
       @lock_args_method ||= :unique_args if worker_method_defined?(:unique_args)

lib/sidekiq_unique_jobs/lock_config.rb

@@ -59,12 +59,12 @@ def self.from_worker(options)
     def initialize(job_hash = {})
       @type        = job_hash[LOCK]&.to_sym
       @worker      = job_hash[CLASS]
-      @limit       = job_hash.fetch(LOCK_LIMIT) { 1 }
-      @timeout     = job_hash.fetch(LOCK_TIMEOUT) { 0 }
-      @ttl         = job_hash.fetch(LOCK_TTL) { job_hash.fetch(LOCK_EXPIRATION) { nil } }.to_i
+      @limit       = job_hash.fetch(LOCK_LIMIT, 1)
+      @timeout     = job_hash.fetch(LOCK_TIMEOUT, 0)
+      @ttl         = job_hash.fetch(LOCK_TTL) { job_hash.fetch(LOCK_EXPIRATION, nil) }.to_i
       @pttl        = ttl * 1_000
       @lock_info   = job_hash.fetch(LOCK_INFO) { SidekiqUniqueJobs.config.lock_info }
-      @on_conflict = job_hash.fetch(ON_CONFLICT) { nil }
+      @on_conflict = job_hash.fetch(ON_CONFLICT, nil)
       @errors      = job_hash.fetch(ERRORS) { {} }
 
       @on_client_conflict = job_hash[ON_CLIENT_CONFLICT]

lib/sidekiq_unique_jobs/logging.rb

@@ -114,8 +114,8 @@ def with_logging_context
     #
     # @yield
     #
-    def with_configured_loggers_context
-      logger_method.call(logging_context) { yield }
+    def with_configured_loggers_context(&block)
+      logger_method.call(logging_context, &block)
     end
 
     #

lib/sidekiq_unique_jobs/middleware/client.rb

@@ -18,15 +18,15 @@ class Client
       #
       # @yield when uniqueness is disable
       # @yield when the lock is successful
-      def call(*)
-        lock { yield }
+      def call(*, &block)
+        lock(&block)
       end
 
       private
 
       def lock
-        if (token = lock_instance.lock)
-          yield token
+        if (_token = lock_instance.lock)
+          yield
         else
           warn_about_duplicate
         end

lib/sidekiq_unique_jobs/middleware/server.rb

@@ -19,8 +19,8 @@ class Server
       #
       # @yield when uniqueness is disabled
       # @yield when owning the lock
-      def call(*)
-        lock_instance.execute { yield }
+      def call(*, &block)
+        lock_instance.execute(&block)
       end
     end
   end

lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb

@@ -151,7 +151,7 @@ def queues(conn, &block)
         conn.sscan_each("queues", &block)
       end
 
-      def entries(conn, queue) # rubocop:disable Metrics/MethodLength
+      def entries(conn, queue, &block) # rubocop:disable Metrics/MethodLength
         queue_key    = "queue:#{queue}"
         initial_size = conn.llen(queue_key)
         deleted_size = 0
@@ -166,9 +166,7 @@ def entries(conn, queue) # rubocop:disable Metrics/MethodLength
 
           break if entries.empty?
 
-          entries.each do |entry|
-            yield entry
-          end
+          entries.each(&block)
 
           deleted_size = initial_size - conn.llen(queue_key)
         end

lib/sidekiq_unique_jobs/sidekiq_worker_methods.rb

@@ -35,7 +35,13 @@ def worker_class
     # The hook to call after a successful unlock
     # @return [Proc]
     def after_unlock_hook
-      -> { worker_class.after_unlock if worker_method_defined?(:after_unlock) }
+      lambda do
+        if @worker_class.respond_to?(:after_unlock)
+          @worker_class.after_unlock # instance method in sidekiq v6
+        elsif worker_class.respond_to?(:after_unlock)
+          worker_class.after_unlock # class method regardless of sidekiq version
+        end
+      end
     end
 
     # Attempt to constantize a string worker_class argument, always

lib/sidekiq_unique_jobs/version_check.rb

@@ -7,10 +7,10 @@ module SidekiqUniqueJobs
   # @author Mikael Henriksson <mikael@zoolutions.se>
   #
   class VersionCheck
-    PATTERN = /(?<operator1>[<>=]+)?\s?(?<version1>(\d+.?)+)(\s+&&\s+)?(?<operator2>[<>=]+)?\s?(?<version2>(\d+.?)+)?/m.freeze # rubocop:disable Layout/LineLength
+    PATTERN = /(?<operator1>[<>=]+)?\s?(?<version1>(\d+.?)+)(\s+&&\s+)?(?<operator2>[<>=]+)?\s?(?<version2>(\d+.?)+)?/m.freeze # rubocop:disable Layout/LineLength, Lint/MixedRegexpCaptureTypes
 
     #
-    # Checks if a version is consrtaint is satisfied
+    # Checks if a version is constraint is satisfied
     #
     # @example A satisfied constraint
     #   VersionCheck.satisfied?("5.0.0", ">= 4.0.0") #=> true
@@ -22,12 +22,31 @@ class VersionCheck
     # @param [String] version a version string `5.0.0`
     # @param [String] constraint a version constraint `>= 5.0.0 <= 5.1.1`
     #
-    # @return [<type>] <description>
+    # @return [true, false] <description>
     #
     def self.satisfied?(version, constraint)
       new(version, constraint).satisfied?
     end
 
+    #
+    # Checks if a version is constraint is unfulfilled
+    #
+    # @example A satisfied constraint
+    #   VersionCheck.unfulfilled?("5.0.0", ">= 4.0.0") #=> false
+    #
+    # @example An unfulfilled constraint
+    #   VersionCheck.unfulfilled?("5.0.0", "<= 4.0.0") #=> true
+    #
+    #
+    # @param [String] version a version string `5.0.0`
+    # @param [String] constraint a version constraint `>= 5.0.0 <= 5.1.1`
+    #
+    # @return [true, false] <description>
+    #
+    def self.unfulfilled?(version, constraint)
+      !satisfied?(version, constraint)
+    end
+
     #
     # @!attribute [r] version
     #   @return [String] a version string `5.0.0`

myapp/bin/check_or_setup_db

@@ -21,39 +21,39 @@ exit begin
   connection_tries ||= 3
   ActiveRecord::Base.establish_connection && ActiveRecord::Migrator.current_version
   0
-     rescue PG::ConnectionBad
-       unless (connection_tries -= 1).zero?
-         puts "Retrying DB connection #{connection_tries} more times..."
-         sleep ENV.fetch("APP_SETUP_WAIT", "5").to_i
-         retry
-       end
-       1
-     rescue ActiveRecord::NoDatabaseError, ActiveRecord::AdapterNotSpecified
-       include ActiveRecord::Tasks # rubocop:disable Style/MixinUsage
-
-       DatabaseTasks.root = File.expand_path "..", __dir__
-       DatabaseTasks.db_dir = File.join DatabaseTasks.root, "db"
-       DatabaseTasks.env = ENV.fetch "ENV", ENV.fetch("RAILS_ENV", "development")
-
-       # The App database seeder:
-       DatabaseTasks.seed_loader = (Class.new do
-         def load_seed
-           seed_file_path = File.join DatabaseTasks.db_dir, "seeds.rb"
-           raise "Seed file '#{seed_file_path}' does not exist" unless File.file?(seed_file_path)
-
-           load seed_file_path
-         end
-       end).new
-
-       # Add model dirs to the autoload_paths for the seeder to run smoothly:
-       ActiveSupport::Dependencies.autoload_paths << File.join(DatabaseTasks.root, "app", "models", "concerns")
-       ActiveSupport::Dependencies.autoload_paths << File.join(DatabaseTasks.root, "app", "models")
-
-       return 2 unless DatabaseTasks.create_current
-       return 3 unless DatabaseTasks.load_schema_current
-       return 4 unless DatabaseTasks.load_seed
-
-       0
-     ensure
-       ActiveRecord::Base.clear_all_connections!
+rescue PG::ConnectionBad
+  unless (connection_tries -= 1).zero?
+    puts "Retrying DB connection #{connection_tries} more times..."
+    sleep ENV.fetch("APP_SETUP_WAIT", "5").to_i
+    retry
+  end
+  1
+rescue ActiveRecord::NoDatabaseError, ActiveRecord::AdapterNotSpecified
+  include ActiveRecord::Tasks
+
+  DatabaseTasks.root = File.expand_path "..", __dir__
+  DatabaseTasks.db_dir = File.join DatabaseTasks.root, "db"
+  DatabaseTasks.env = ENV.fetch "ENV", ENV.fetch("RAILS_ENV", "development")
+
+  # The App database seeder:
+  DatabaseTasks.seed_loader = (Class.new do
+    def load_seed
+      seed_file_path = File.join DatabaseTasks.db_dir, "seeds.rb"
+      raise "Seed file '#{seed_file_path}' does not exist" unless File.file?(seed_file_path)
+
+      load seed_file_path
+    end
+  end).new
+
+  # Add model dirs to the autoload_paths for the seeder to run smoothly:
+  ActiveSupport::Dependencies.autoload_paths << File.join(DatabaseTasks.root, "app", "models", "concerns")
+  ActiveSupport::Dependencies.autoload_paths << File.join(DatabaseTasks.root, "app", "models")
+
+  exit 2 unless DatabaseTasks.create_current
+  exit 3 unless DatabaseTasks.load_schema_current
+  exit 4 unless DatabaseTasks.load_seed
+
+  0
+ensure
+  ActiveRecord::Base.clear_all_connections!
 end

myapp/cable.ru

@@ -2,7 +2,7 @@
 
 # This file is used by Rack-based servers to start the application.
 
-require ::File.expand_path("../config/environment", __FILE__)
+require ::File.expand_path("config/environment", __dir__)
 Rails.application.eager_load!
 
 run ActionCable.server

myapp/config.ru

@@ -2,6 +2,6 @@
 
 # This file is used by Rack-based servers to start the application.
 
-require ::File.expand_path("../config/environment", __FILE__)
+require ::File.expand_path("config/environment", __dir__)
 
 run Rails.application

myapp/config/environments/production.rb

@@ -83,7 +83,7 @@
   # config.logger = ActiveSupport::TaggedLogging.new(Syslog::Logger.new 'app-name')
 
   if ENV["RAILS_LOG_TO_STDOUT"].present?
-    logger           = ActiveSupport::Logger.new(STDOUT)
+    logger           = ActiveSupport::Logger.new($stdout)
     logger.formatter = config.log_formatter
     config.logger    = ActiveSupport::TaggedLogging.new(logger)
   end

myapp/config/initializers/sidekiq.rb

@@ -43,7 +43,7 @@
   Sidekiq::Status.configure_client_middleware config, expiration: 30.minutes
 end
 
-Sidekiq.logger       = Sidekiq::Logger.new(STDOUT)
+Sidekiq.logger       = Sidekiq::Logger.new($stdout)
 Sidekiq.logger.level = Logger::DEBUG
 Sidekiq.log_format = :json if Sidekiq.respond_to?(:log_format)
 SidekiqUniqueJobs.configure do |config|

myapp/config/puma.rb

@@ -6,20 +6,20 @@
 # the maximum value specified for Puma. Default is set to 5 threads for minimum
 # and maximum; this matches the default thread size of Active Record.
 #
-max_threads_count = ENV.fetch("RAILS_MAX_THREADS") { 5 }
+max_threads_count = ENV.fetch("RAILS_MAX_THREADS", 5)
 min_threads_count = ENV.fetch("RAILS_MIN_THREADS") { max_threads_count }
 threads min_threads_count, max_threads_count
 
 # Specifies the `port` that Puma will listen on to receive requests; default is 3000.
 #
-port        ENV.fetch("PORT") { 3000 }
+port        ENV.fetch("PORT", 3000)
 
 # Specifies the `environment` that Puma will run in.
 #
-environment ENV.fetch("RAILS_ENV") { "development" }
+environment ENV.fetch("RAILS_ENV", "development")
 
 # Specifies the `pidfile` that Puma will use.
-pidfile ENV.fetch("PIDFILE") { "tmp/pids/server.pid" }
+pidfile ENV.fetch("PIDFILE", "tmp/pids/server.pid")
 
 # Specifies the number of `workers` to boot in clustered mode.
 # Workers are forked web server processes. If using threads and workers together