From e9847119a05d6da2b7c7f6ac615c55dbd3a6ee8b Mon Sep 17 00:00:00 2001 From: Rory Hughes Date: Wed, 1 Aug 2018 19:13:18 -0400 Subject: [PATCH] Fix replace conflict strategy After initially failing to get this feature working, I found that the strategy class wasn't in the hash of strategies. The other thing this PR addresses is removing the client middleware is loaded on the server. The client middleware checks if a lock is in place and enqueues in its absense. Client middleware is only hit when the server enqueues to itself which happens when sidekiq takes scheduled items and enqueues them on the work queue. I was trying to use the `:until_executing` lock along with `:replace` strategy, but the server would just take the job and drop in `Client::Middleware#call` because the lock only gets released during the server middleware call. All ears if the client middleware on the server is necessary for whatever other reason. All the tests pass without it (bar the test which just checks the presence of it, which I fixed). --- lib/sidekiq_unique_jobs/middleware.rb | 5 ----- lib/sidekiq_unique_jobs/on_conflict.rb | 1 + spec/unit/sidekiq_unique_jobs/middleware_spec.rb | 3 --- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/lib/sidekiq_unique_jobs/middleware.rb b/lib/sidekiq_unique_jobs/middleware.rb index 3e72aa4bf..4720c343d 100644 --- a/lib/sidekiq_unique_jobs/middleware.rb +++ b/lib/sidekiq_unique_jobs/middleware.rb @@ -17,11 +17,6 @@ def configure_middleware def configure_server_middleware Sidekiq.configure_server do |config| - config.client_middleware do |chain| - require 'sidekiq_unique_jobs/client/middleware' - chain.add SidekiqUniqueJobs::Client::Middleware - end - config.server_middleware do |chain| require 'sidekiq_unique_jobs/server/middleware' chain.add SidekiqUniqueJobs::Server::Middleware diff --git a/lib/sidekiq_unique_jobs/on_conflict.rb b/lib/sidekiq_unique_jobs/on_conflict.rb index 1ebc6d25d..7dd390a67 100644 --- a/lib/sidekiq_unique_jobs/on_conflict.rb +++ b/lib/sidekiq_unique_jobs/on_conflict.rb @@ -14,6 +14,7 @@ module OnConflict log: OnConflict::Log, raise: OnConflict::Raise, reject: OnConflict::Reject, + replace: OnConflict::Replace, reschedule: OnConflict::Reschedule, }.freeze diff --git a/spec/unit/sidekiq_unique_jobs/middleware_spec.rb b/spec/unit/sidekiq_unique_jobs/middleware_spec.rb index 82cb16d73..d2a77b038 100644 --- a/spec/unit/sidekiq_unique_jobs/middleware_spec.rb +++ b/spec/unit/sidekiq_unique_jobs/middleware_spec.rb @@ -21,9 +21,6 @@ it 'adds client and server middleware when required' do expect(Sidekiq).to receive(:configure_server).and_yield(server_config) - expect(server_config).to receive(:client_middleware).and_yield(client_middleware) - expect(client_middleware).to receive(:add).with(SidekiqUniqueJobs::Client::Middleware) - expect(server_config).to receive(:server_middleware).and_yield(server_middleware) expect(server_middleware).to receive(:add).with(SidekiqUniqueJobs::Server::Middleware) described_class.configure_server_middleware