From d985aa35f04f532a6d000e4929e1052824a4f78d Mon Sep 17 00:00:00 2001 From: Mike Perham Date: Mon, 6 Jun 2022 12:07:51 -0700 Subject: [PATCH] Adjust middleware, fixes #5371 Middleware now has access to the `config` object so we can provide local helper methods. --- Changes.md | 9 +- docs/middleware.md | 87 ++++++++++++++++++++ lib/sidekiq.rb | 4 +- lib/sidekiq/middleware/chain.rb | 31 ++++--- lib/sidekiq/middleware/current_attributes.rb | 4 + lib/sidekiq/middleware/i18n.rb | 2 + lib/sidekiq/middleware/modules.rb | 19 +++++ test/test_middleware.rb | 24 ++++++ 8 files changed, 163 insertions(+), 17 deletions(-) create mode 100644 docs/middleware.md create mode 100644 lib/sidekiq/middleware/modules.rb diff --git a/Changes.md b/Changes.md index f00a5401..11a9a2b1 100644 --- a/Changes.md +++ b/Changes.md @@ -5,11 +5,12 @@ 6.5.0 --------- -- Substantial refactoring of Sidekiq server internals, as part of a larger effort to - reduce Sidekiq's internal usage of global methods and data, see [docs/component.md](docs/component.md) and [docs/global_to_local.md](docs/global_to_local.md). -- Add **beta** support for the `redis-client` gem**. This will become the default Redis driver in Sidekiq 7.0. [#5298] +- Substantial refactoring of Sidekiq server internals, part of a larger effort + to reduce Sidekiq's internal usage of global methods and data, see [docs/component.md](docs/component.md), + [docs/global_to_local.md](docs/global_to_local.md) and [docs/middleware.md](docs/middleware.md). +- **Add beta support for the `redis-client` gem**. This will become the default Redis driver in Sidekiq 7.0. [#5298] Read more: https://github.com/mperham/sidekiq/wiki/Using-redis-client -- Add **beta** support for DB transaction-aware client [#5291] +- **Add beta support for DB transaction-aware client** [#5291] Add this line to your initializer and any jobs created during a transaction will only be pushed to Redis **after the transaction commits**. You will need to add the `after_commit_everywhere` gem to your Gemfile. diff --git a/docs/middleware.md b/docs/middleware.md new file mode 100644 index 00000000..bd56089f --- /dev/null +++ b/docs/middleware.md @@ -0,0 +1,87 @@ +# Middleware Changes in Sidekiq 7.0 + +With the internal refactoring coming in Sidekiq 7.0 it is necessary +to make minor changes to the Middleware API. + +> tl;dr - middleware should now include Sidekiq::ClientMiddleware or Sidekiq::ServerMiddleware. + +Currently the middleware API looks like this: + +## Existing Client API + +Client middleware is run when pushing a job to Redis. + +```ruby +class Client + def initialize(optional_args) + @args = optional_args + end + def call(worker, job, queue, redis_pool) + yield + end +end + +Sidekiq.configure_client do |config| + config.client_middleware do |chain| + chain.add Client, optional_args + end +end +``` + +## Server + +Server middleware is run around job execution. + +```ruby +class Server + def initialize(optional_args) + @args = optional_args + end + def call(worker, job, queue) + Sidekiq.redis {|c| c.do_something } + Sidekiq.logger.info { "Some message" } + yield + end +end + +Sidekiq.configure_server do |config| + config.server_middleware do |chain| + chain.add Server, optional_args + end +end +``` + +## Updated API + +The updated middleware API requires the middleware class to include +a helper module. + +```ruby +class Client + include Sidekiq::ClientMiddleware + + def initialize(optional_args) + @args = optional_args + end + def call(worker, job, queue, redis_pool) + yield + end +end +``` + +```ruby +class Server + include Sidekiq::ServerMiddleware + + def initialize(optional_args) + @args = optional_args + end + def call(worker, job, queue) + # note we no longer need to use the global Sidekiq module + # to access Redis and the logger + redis {|c| c.do_something } + logger.info { "Some message" } + yield + end +end +``` \ No newline at end of file diff --git a/lib/sidekiq.rb b/lib/sidekiq.rb index 30fb1cb7..b084d687 100644 --- a/lib/sidekiq.rb +++ b/lib/sidekiq.rb @@ -199,7 +199,7 @@ module Sidekiq end def self.client_middleware - @client_chain ||= Middleware::Chain.new + @client_chain ||= Middleware::Chain.new(self) yield @client_chain if block_given? @client_chain end @@ -211,7 +211,7 @@ module Sidekiq end def self.default_server_middleware - Middleware::Chain.new + Middleware::Chain.new(self) end def self.default_worker_options=(hash) # deprecated diff --git a/lib/sidekiq/middleware/chain.rb b/lib/sidekiq/middleware/chain.rb index 2a4210e9..d19a21ef 100644 --- a/lib/sidekiq/middleware/chain.rb +++ b/lib/sidekiq/middleware/chain.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "sidekiq/middleware/modules" + module Sidekiq # Middleware is code configured to run before/after # a message is processed. It is patterned after Rack @@ -44,10 +46,12 @@ module Sidekiq # This is an example of a minimal server middleware: # # class MyServerHook + # include Sidekiq::ServerMiddleware # def call(job_instance, msg, queue) - # puts "Before job" + # logger.info "Before job" + # redis {|conn| conn.get("foo") } # do something in Redis # yield - # puts "After job" + # logger.info "After job" # end # end # @@ -56,10 +60,11 @@ module Sidekiq # to Redis: # # class MyClientHook + # include Sidekiq::ClientMiddleware # def call(job_class, msg, queue, redis_pool) - # puts "Before push" + # logger.info "Before push" # result = yield - # puts "After push" + # logger.info "After push" # result # end # end @@ -76,7 +81,8 @@ module Sidekiq entries.each(&block) end - def initialize + def initialize(config = nil) + @config = config @entries = nil yield self if block_given? end @@ -91,24 +97,24 @@ module Sidekiq def add(klass, *args) remove(klass) - entries << Entry.new(klass, *args) + entries << Entry.new(@config, klass, *args) end def prepend(klass, *args) remove(klass) - entries.insert(0, Entry.new(klass, *args)) + entries.insert(0, Entry.new(@config, klass, *args)) end def insert_before(oldklass, newklass, *args) i = entries.index { |entry| entry.klass == newklass } - new_entry = i.nil? ? Entry.new(newklass, *args) : entries.delete_at(i) + new_entry = i.nil? ? Entry.new(@config, newklass, *args) : entries.delete_at(i) i = entries.index { |entry| entry.klass == oldklass } || 0 entries.insert(i, new_entry) end def insert_after(oldklass, newklass, *args) i = entries.index { |entry| entry.klass == newklass } - new_entry = i.nil? ? Entry.new(newklass, *args) : entries.delete_at(i) + new_entry = i.nil? ? Entry.new(@config, newklass, *args) : entries.delete_at(i) i = entries.index { |entry| entry.klass == oldklass } || entries.count - 1 entries.insert(i + 1, new_entry) end @@ -149,13 +155,16 @@ module Sidekiq class Entry attr_reader :klass - def initialize(klass, *args) + def initialize(config, klass, *args) + @config = config @klass = klass @args = args end def make_new - @klass.new(*@args) + x = @klass.new(*@args) + x.config = @config if @config && x.respond_to?(:config=) + x end end end diff --git a/lib/sidekiq/middleware/current_attributes.rb b/lib/sidekiq/middleware/current_attributes.rb index f62b587b..7167b51f 100644 --- a/lib/sidekiq/middleware/current_attributes.rb +++ b/lib/sidekiq/middleware/current_attributes.rb @@ -15,6 +15,8 @@ module Sidekiq # module CurrentAttributes class Save + include Sidekiq::ClientMiddleware + def initialize(cattr) @klass = cattr end @@ -31,6 +33,8 @@ module Sidekiq end class Load + include Sidekiq::ServerMiddleware + def initialize(cattr) @klass = cattr end diff --git a/lib/sidekiq/middleware/i18n.rb b/lib/sidekiq/middleware/i18n.rb index 9230172e..05357cfd 100644 --- a/lib/sidekiq/middleware/i18n.rb +++ b/lib/sidekiq/middleware/i18n.rb @@ -10,6 +10,7 @@ module Sidekiq::Middleware::I18n # Get the current locale and store it in the message # to be sent to Sidekiq. class Client + include Sidekiq::ClientMiddleware def call(_jobclass, job, _queue, _redis) job["locale"] ||= I18n.locale yield @@ -18,6 +19,7 @@ module Sidekiq::Middleware::I18n # Pull the msg locale out and set the current thread to use it. class Server + include Sidekiq::ServerMiddleware def call(_jobclass, job, _queue, &block) I18n.with_locale(job.fetch("locale", I18n.default_locale), &block) end diff --git a/lib/sidekiq/middleware/modules.rb b/lib/sidekiq/middleware/modules.rb new file mode 100644 index 00000000..6ed875d1 --- /dev/null +++ b/lib/sidekiq/middleware/modules.rb @@ -0,0 +1,19 @@ +module Sidekiq + module ServerMiddleware + attr_accessor :config + def redis_pool + config.redis_pool + end + + def logger + config.logger + end + + def redis(&block) + config.redis(&block) + end + end + + # no difference for now + ClientMiddleware = ServerMiddleware +end diff --git a/test/test_middleware.rb b/test/test_middleware.rb index a6b51c2e..2a360f8a 100644 --- a/test/test_middleware.rb +++ b/test/test_middleware.rb @@ -165,4 +165,28 @@ describe Sidekiq::Middleware do I18n.available_locales = nil end end + + class FooC + include Sidekiq::ClientMiddleware + def initialize(*args) + @args = args + end + + def call(w, j, q, rp) + redis { |c| c.incr(self.class.name) } + logger.info { |c| [self.class.name, @args].inspect } + yield + end + end + + describe "configuration" do + it "gets an object which provides redis and logging" do + cfg = Sidekiq + chain = Sidekiq::Middleware::Chain.new(cfg) + chain.add FooC, foo: "bar" + final_action = nil + chain.invoke(nil, nil, nil, nil) { final_action = true } + assert final_action + end + end end