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

Support both instance method and class method #527

Merged
merged 8 commits into from
Sep 27, 2020
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
1 change: 1 addition & 0 deletions .reek.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ inherit_gem:

AllCops:
TargetRubyVersion: 2.5
NewCops: enable
Include:
- "examples/**/*.rb"
Exclude:
- "**/*.erb"
- "**/*.lua"
- "myapp"
- "bin/bench"

Layout/EndAlignment:
Expand Down Expand Up @@ -91,6 +93,9 @@ RSpec/InstanceVariable:
RSpec/MultipleExpectations:
Enabled: false

RSpec/MultipleMemoizedHelpers:
Enabled: false

RSpec/NestedGroups:
Max: 4
Enabled: true
Expand Down
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
2 changes: 1 addition & 1 deletion bin/bundle
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
1 change: 0 additions & 1 deletion lib/sidekiq_unique_jobs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion lib/sidekiq_unique_jobs/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion lib/sidekiq_unique_jobs/lock_args.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions lib/sidekiq_unique_jobs/lock_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
4 changes: 2 additions & 2 deletions lib/sidekiq_unique_jobs/logging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

#
Expand Down
8 changes: 4 additions & 4 deletions lib/sidekiq_unique_jobs/middleware/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/sidekiq_unique_jobs/middleware/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 7 additions & 1 deletion lib/sidekiq_unique_jobs/sidekiq_worker_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 22 additions & 3 deletions lib/sidekiq_unique_jobs/version_check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ module SidekiqUniqueJobs
# @author Mikael Henriksson <[email protected]>
#
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
Expand All @@ -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`
Expand Down
70 changes: 35 additions & 35 deletions myapp/bin/check_or_setup_db
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion myapp/cable.ru
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion myapp/config.ru
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion myapp/config/environments/production.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion myapp/config/initializers/sidekiq.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
8 changes: 4 additions & 4 deletions myapp/config/puma.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading