Rename lock_args to lock_args_method (mhenrixon#551)

* Fix lock_args not being set

* Mandatory rubocop commit

* Make lock_args idempotent

Otherwise, lock_args are redefined on every run both from server and client.

* Add more to post-install

* Mandatory rubocop commit

* Add regression tests and fix rubocop violations

* Add lock_args => lock_args_method to deprecation

This should allow people who use the worker validation to catch this.

* Adds missing coverage

.gitignore

@@ -21,3 +21,4 @@ tmp/
 /Gemfile.local
 /gemfiles/vendor/
 /vendor/
+/myapp/vendor/bundle/

.rubocop.yml

@@ -10,7 +10,7 @@ AllCops:
   Exclude:
     - "**/*.erb"
     - "**/*.lua"
-    - "myapp"
+    - '**/vendor/**/*'
     - "bin/bench"
 
 Layout/EndAlignment:

README.md

@@ -536,7 +536,7 @@ The method or the proc can return a modified version of args without the transie
 class UniqueJobWithFilterMethod
   include Sidekiq::Worker
   sidekiq_options lock: :until_and_while_executing,
-                  lock_args: :lock_args # this is default and will be used if such a method is defined
+                  lock_args_method: :lock_args # this is default and will be used if such a method is defined
 
   def self.lock_args(args)
     [ args[0], args[2][:type] ]
@@ -549,7 +549,7 @@ end
 class UniqueJobWithFilterProc
   include Sidekiq::Worker
   sidekiq_options lock: :until_executed,
-                  lock_args: ->(args) { [ args.first ] }
+                  lock_args_method: ->(args) { [ args.first ] }
 
   ...
 
@@ -561,7 +561,7 @@ It is possible to ensure different types of unique args based on context. I can'
 ```ruby
 class UniqueJobWithFilterMethod
   include Sidekiq::Worker
-  sidekiq_options lock: :until_and_while_executing, lock_args: :lock_args
+  sidekiq_options lock: :until_and_while_executing, lock_args_method: :lock_args
 
   def self.lock_args(args)
     if Sidekiq::ProcessSet.new.size > 1

lib/sidekiq_unique_jobs/constants.rb

@@ -19,6 +19,7 @@ module SidekiqUniqueJobs
   LIVE_VERSION          ||= "uniquejobs:live"
   LOCK                  ||= "lock"
   LOCK_ARGS             ||= "lock_args"
+  LOCK_ARGS_METHOD      ||= "lock_args_method"
   LOCK_DIGEST           ||= "lock_digest"
   LOCK_EXPIRATION       ||= "lock_expiration"
   LOCK_INFO             ||= "lock_info"
@@ -43,6 +44,7 @@ module SidekiqUniqueJobs
   UNIQUE_ACROSS_QUEUES  ||= "unique_across_queues"
   UNIQUE_ACROSS_WORKERS ||= "unique_across_workers"
   UNIQUE_ARGS           ||= "unique_args"
+  UNIQUE_ARGS_METHOD    ||= "unique_args_method"
   UNIQUE_DIGEST         ||= "unique_digest"
   UNIQUE_PREFIX         ||= "unique_prefix"
   UNIQUE_REAPER         ||= "uniquejobs:reaper"

lib/sidekiq_unique_jobs/lock/validator.rb

@@ -12,7 +12,8 @@ class Validator
       # @return [Hash] a hash mapping of deprecated keys and their new value
       DEPRECATED_KEYS = {
         UNIQUE.to_sym => LOCK.to_sym,
-        UNIQUE_ARGS.to_sym => LOCK_ARGS.to_sym,
+        UNIQUE_ARGS.to_sym => LOCK_ARGS_METHOD.to_sym,
+        LOCK_ARGS.to_sym => LOCK_ARGS_METHOD.to_sym,
         UNIQUE_PREFIX.to_sym => LOCK_PREFIX.to_sym,
       }.freeze
 

lib/sidekiq_unique_jobs/lock_args.rb

@@ -97,17 +97,17 @@ def filter_by_symbol(args)
     end
 
     # The method to use for filtering unique arguments
-    def lock_args_method # rubocop:disable Metrics/CyclomaticComplexity
-      @lock_args_method ||= worker_options[LOCK_ARGS] || worker_options[UNIQUE_ARGS]
+    def lock_args_method
+      @lock_args_method ||= worker_options.slice(LOCK_ARGS_METHOD, UNIQUE_ARGS_METHOD).values.first
       @lock_args_method ||= :lock_args if worker_method_defined?(:lock_args)
       @lock_args_method ||= :unique_args if worker_method_defined?(:unique_args)
       @lock_args_method ||= default_lock_args_method
     end
 
     # The global worker options defined in Sidekiq directly
     def default_lock_args_method
-      default_worker_options[LOCK_ARGS] ||
-        default_worker_options[UNIQUE_ARGS]
+      default_worker_options[LOCK_ARGS_METHOD] ||
+        default_worker_options[UNIQUE_ARGS_METHOD]
     end
 
     #

lib/sidekiq_unique_jobs/sidekiq_worker_methods.rb

@@ -50,7 +50,7 @@ def after_unlock_hook
     # @return [Sidekiq::Worker]
     def worker_class_constantize(klazz = @worker_class)
       return klazz.class if klazz.is_a?(Sidekiq::Worker) # sidekiq v6.x
-      return klazz       unless klazz.is_a?(String)
+      return klazz unless klazz.is_a?(String)
 
       Object.const_get(klazz)
     rescue NameError => ex

myapp/app/workers/until_executed_with_lock_args_job.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+class UntilExecutedWithLockArgsJob
+  include Sidekiq::Worker
+
+  sidekiq_options lock: :until_executed,
+                  lock_args_method: :lock_args,
+                  lock_timeout: 0,
+                  lock_ttl: 600,
+                  lock_limit: 2
+
+  def self.lock_args(args)
+    options = args.extract_options!
+    [args[0], options["bogus"]]
+  end
+
+  def perform(*_args)
+    logger.info("cowboy")
+    sleep(1)
+    logger.info("beebop")
+  end
+end

sidekiq-unique-jobs.gemspec

@@ -29,7 +29,19 @@ Gem::Specification.new do |spec|
   spec.metadata["changelog_uri"]     = "https://github.com/mhenrixon/sidekiq-unique-jobs/blob/master/CHANGELOG.md"
 
   spec.post_install_message = <<~POST_INSTALL
-    This version deprecated the configuration options:
+    IMPORTANT!
+
+    This version deprecated the following sidekiq_options
+
+      - sidekiq_options lock_args: :method_name
+
+    It is now configured with:
+
+      - sidekiq_options lock_args_method: :method_name
+
+    This is also true for `Sidekiq.default_worker_options`
+
+    We also deprecated the global configuration options:
       - default_lock_ttl
       - default_lock_ttl=
       - default_lock_timeout

spec/sidekiq_unique_jobs/lock/validator_spec.rb

@@ -23,13 +23,15 @@
         {
           "unique" => "until_executed",
           "unique_args" => "hokus",
+          "lock_args" => "hokus",
           "unique_prefix" => "pokus",
         }
       end
 
       it "writes a helpful message about the deprecated key" do
         expect(validate.errors[:unique]).to eq("is deprecated, use `lock: until_executed` instead.")
-        expect(validate.errors[:unique_args]).to eq("is deprecated, use `lock_args: hokus` instead.")
+        expect(validate.errors[:unique_args]).to eq("is deprecated, use `lock_args_method: hokus` instead.")
+        expect(validate.errors[:lock_args]).to eq("is deprecated, use `lock_args_method: hokus` instead.")
         expect(validate.errors[:unique_prefix]).to eq("is deprecated, use `lock_prefix: pokus` instead.")
       end
     end

spec/sidekiq_unique_jobs/lock_args_spec.rb

@@ -18,32 +18,32 @@
     subject(:lock_args_enabled?) { lock_args.lock_args_enabled? }
 
     context "with default worker options", :with_sidekiq_options do
-      let(:sidekiq_options) { { unique: :until_executed, lock_args: ->(args) { args[1]["test"] } } }
+      let(:sidekiq_options) { { unique: :until_executed, lock_args_method: ->(args) { args[1]["test"] } } }
 
-      context "when `lock_args: :lock_args` in worker", :with_worker_options do
-        let(:worker_options) { { lock_args: :lock_args } }
+      context "when `lock_args_method: :lock_args` in worker", :with_worker_options do
+        let(:worker_options) { { lock_args_method: :lock_args } }
 
         it { is_expected.to eq(:lock_args) }
       end
 
-      context "when `lock_args: false` in worker", :with_worker_options do
-        let(:worker_options) { { lock_args: false } }
+      context "when `lock_args_method: false` in worker", :with_worker_options do
+        let(:worker_options) { { lock_args_method: false } }
 
         it { is_expected.to be_a(Proc) }
       end
     end
 
     context "when disabled in default_worker_options", :with_sidekiq_options do
-      let(:sidekiq_options) { { unique: false, lock_args: nil } }
+      let(:sidekiq_options) { { unique: false, lock_args_method: nil } }
 
-      context "when `lock_args: :lock_args` in worker", :with_worker_options do
-        let(:worker_options) { { lock_args: :lock_args } }
+      context "when `lock_args_method: :lock_args` in worker", :with_worker_options do
+        let(:worker_options) { { lock_args_method: :lock_args } }
 
         it { is_expected.to eq(:lock_args) }
       end
 
-      context "when `lock_args: false` in worker", :with_worker_options do
-        let(:worker_options) { { lock_args: false } }
+      context "when `lock_args_method: false` in worker", :with_worker_options do
+        let(:worker_options) { { lock_args_method: false } }
 
         it { is_expected.to eq(nil) }
       end
@@ -79,7 +79,7 @@
 
     context "when configured globally" do
       it "uses global filter" do
-        Sidekiq.use_options(lock_args: ->(args) { args.first }) do
+        Sidekiq.use_options(lock_args_method: ->(args) { args.first }) do
           expect(filter_by_proc).to eq(1)
         end
       end

spec/sidekiq_unique_jobs/middleware/client_spec.rb

@@ -97,11 +97,11 @@
     end
   end
 
-  describe "when unique_args is defined" do
+  describe "when lock_args_method is defined" do
     context "when filter method is defined" do
       it "pushes no duplicate messages" do
         expect(CustomQueueJobWithFilterMethod).to respond_to(:args_filter)
-        expect(CustomQueueJobWithFilterMethod.get_sidekiq_options["lock_args"]).to eq :args_filter
+        expect(CustomQueueJobWithFilterMethod.get_sidekiq_options["lock_args_method"]).to eq :args_filter
 
         Array.new(10) do |i|
           push_item(
@@ -112,16 +112,20 @@
         end
 
         expect(queue_count("customqueue")).to eq(1)
+        # NOTE: Below is for regression purposes
+        expect(Sidekiq::Queue.new("customqueue").entries.first.item["lock_args"]).to eq(1)
       end
     end
 
     context "when filter proc is defined" do
-      let(:args) { [1, { random: rand, name: "foobar" }] }
+      let(:args) { [1, { "random" => rand, "name" => "foobar" }] }
 
       it "pushes no duplicate messsages" do
-        Array.new(100) { CustomQueueJobWithFilterProc.perform_async(args) }
+        Array.new(100) { CustomQueueJobWithFilterProc.perform_async(*args) }
 
         expect(queue_count("customqueue")).to eq(1)
+        # NOTE: Below is for regression purposes
+        expect(Sidekiq::Queue.new("customqueue").entries.first.item["lock_args"]).to eq([1, "foobar"])
       end
     end
 

spec/support/workers/custom_queue_job_with_filter_method.rb

@@ -5,7 +5,7 @@
 require_relative "custom_queue_job"
 
 class CustomQueueJobWithFilterMethod < CustomQueueJob
-  sidekiq_options lock: :until_executed, lock_args: :args_filter
+  sidekiq_options lock: :until_executed, lock_args_method: :args_filter
 
   def self.args_filter(args)
     args.first

spec/support/workers/custom_queue_job_with_filter_proc.rb

@@ -8,9 +8,7 @@ class CustomQueueJobWithFilterProc < CustomQueueJob
   # slightly contrived example of munging args to the
   # worker and removing a random bit.
   sidekiq_options lock: :until_expired,
-                  lock_args: (lambda do |args|
-                    options = args.extract_options!
-                    options.delete("random")
-                    args + [options]
+                  lock_args_method: (lambda do |args|
+                    [args[0], args[1]["name"]]
                   end)
 end

spec/support/workers/my_unique_job_with_filter_method.rb

@@ -8,13 +8,13 @@ class MyUniqueJobWithFilterMethod
                   lock: :until_executed,
                   queue: :customqueue,
                   retry: true,
-                  lock_args: :filtered_args
+                  lock_args_method: :lock_args
 
   def perform(*)
     # NO-OP
   end
 
-  def self.filtered_args(args)
+  def self.lock_args(args)
     options = args.extract_options!
     [args.first, options["type"]]
   end

spec/support/workers/my_unique_job_with_filter_proc.rb

@@ -8,7 +8,7 @@ class MyUniqueJobWithFilterProc
                   lock: :until_executed,
                   queue: :customqueue,
                   retry: true,
-                  lock_args: (lambda do |args|
+                  lock_args_method: (lambda do |args|
                     options = args.extract_options!
                     [args.first, options["type"]]
                   end)

spec/support/workers/simple_worker.rb

@@ -6,7 +6,7 @@ class SimpleWorker
   include Sidekiq::Worker
   sidekiq_options lock: :until_executed,
                   queue: :default,
-                  lock_args: ->(args) { [args.first] }
+                  lock_args_method: ->(args) { [args.first] }
 
   def perform(args)
     sleep 5

spec/support/workers/unique_job_with_conditional_parameter.rb

@@ -8,7 +8,7 @@ class UniqueJobWithoutUniqueArgsParameter
                   lock: :until_executed,
                   queue: :customqueue,
                   retry: true,
-                  lock_args: :unique_args
+                  lock_args_method: :unique_args
 
   def perform(conditional = nil)
     [conditional]

spec/support/workers/unique_job_with_filter_method.rb

@@ -8,13 +8,13 @@ class UniqueJobWithFilterMethod
                   lock: :while_executing,
                   queue: :customqueue,
                   retry: 1,
-                  lock_args: :filtered_args
+                  lock_args_method: :lock_args
 
   def perform(*)
     # NO-OP
   end
 
-  def self.filtered_args(args)
+  def self.lock_args(args)
     options = args.extract_options!
     [args.first, options["type"]]
   end

spec/support/workers/unique_job_with_nil_unique_args.rb

@@ -8,7 +8,7 @@ class UniqueJobWithNilUniqueArgs
                   lock: :until_executed,
                   queue: :customqueue,
                   retry: true,
-                  lock_args: :unique_args
+                  lock_args_method: :unique_args
 
   def perform(args)
     [args]

spec/support/workers/unique_job_with_no_unique_args_method.rb

@@ -8,7 +8,7 @@ class UniqueJobWithNoUniqueArgsMethod
                   lock: :until_executed,
                   queue: :customqueue,
                   retry: true,
-                  lock_args: :filtered_args
+                  lock_args_method: :filtered_args
 
   def perform(one, two)
     [one, two]

spec/support/workers/unique_job_without_unique_args_parameter.rb

@@ -8,7 +8,7 @@ class UniqueJobWithoutUniqueArgsParameter
                   lock: :until_executed,
                   queue: :customqueue,
                   retry: true,
-                  lock_args: :unique_args
+                  lock_args_method: :unique_args
 
   def perform(optional = true) # rubocop:disable Style/OptionalBooleanParameter
     optional

spec/workers/custom_queue_job_with_filter_method_spec.rb

@@ -7,7 +7,7 @@
         "queue" => :customqueue,
         "retry" => true,
         "lock" => :until_executed,
-        "lock_args" => :args_filter,
+        "lock_args_method" => :args_filter,
       }
     end
   end

spec/workers/custom_queue_job_with_filter_proc_spec.rb

@@ -7,7 +7,7 @@
         "queue" => :customqueue,
         "retry" => true,
         "lock" => :until_expired,
-        "lock_args" => a_kind_of(Proc),
+        "lock_args_method" => a_kind_of(Proc),
       }
     end
   end

spec/workers/my_unique_job_with_filter_method_spec.rb

@@ -8,7 +8,7 @@
         "queue" => :customqueue,
         "retry" => true,
         "lock" => :until_executed,
-        "lock_args" => :filtered_args,
+        "lock_args_method" => :lock_args,
       }
     end
   end
@@ -17,8 +17,8 @@
     let(:args) { ["hundred", { "type" => "extremely unique", "id" => 44 }] }
   end
 
-  describe ".filtered_args" do
-    subject { described_class.filtered_args(args) }
+  describe ".lock_args" do
+    subject { described_class.lock_args(args) }
 
     let(:args) { ["two", { "type" => "very unique", "id" => 4 }] }