From 6be902e8b91eedbfe8760418ab13ed663dce6e93 Mon Sep 17 00:00:00 2001 From: Karol Galanciak Date: Fri, 24 Jul 2026 11:55:56 +0200 Subject: [PATCH 1/4] Add optional operations/sidekiq async execution integration Extract the Sidekiq machinery for running operations asynchronously into the gem itself: Operations::Sidekiq::Convenience/Command/Job/Serializer/ Deserializer plus the operation_container helper. The integration is an opt-in require ("operations/sidekiq"), the Job is Sentry-free with an overridable #instrument hook, and sidekiq/globalid/money are dev-only dependencies. Adds self-contained specs and README/CHANGELOG docs. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + README.md | 77 +++++++++++ lib/operations/sidekiq.rb | 44 ++++++ lib/operations/sidekiq/command.rb | 73 ++++++++++ lib/operations/sidekiq/convenience.rb | 69 ++++++++++ lib/operations/sidekiq/deserializer.rb | 66 +++++++++ lib/operations/sidekiq/job.rb | 69 ++++++++++ lib/operations/sidekiq/serializer.rb | 91 ++++++++++++ operations.gemspec | 7 + spec/operations/sidekiq/command_spec.rb | 131 ++++++++++++++++++ spec/operations/sidekiq/convenience_spec.rb | 138 +++++++++++++++++++ spec/operations/sidekiq/deserializer_spec.rb | 71 ++++++++++ spec/operations/sidekiq/serializer_spec.rb | 105 ++++++++++++++ spec/operations/sidekiq_spec.rb | 62 +++++++++ spec/spec_helper.rb | 17 +++ 15 files changed, 1021 insertions(+) create mode 100644 lib/operations/sidekiq.rb create mode 100644 lib/operations/sidekiq/command.rb create mode 100644 lib/operations/sidekiq/convenience.rb create mode 100644 lib/operations/sidekiq/deserializer.rb create mode 100644 lib/operations/sidekiq/job.rb create mode 100644 lib/operations/sidekiq/serializer.rb create mode 100644 spec/operations/sidekiq/command_spec.rb create mode 100644 spec/operations/sidekiq/convenience_spec.rb create mode 100644 spec/operations/sidekiq/deserializer_spec.rb create mode 100644 spec/operations/sidekiq/serializer_spec.rb create mode 100644 spec/operations/sidekiq_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 85c26e8..8a781ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Added +- Add optional `operations/sidekiq` integration for asynchronous operation execution: `Operations::Sidekiq::Convenience`, `Operations::Sidekiq::Command`, `Operations::Sidekiq::Job`, and the `Serializer`/`Deserializer` that encode rich argument types over Sidekiq. - Allow receiving params in preconditions. [\#56](https://github.com/BookingSync/operations/pull/56) ([pyromaniac](https://github.com/pyromaniac)) ### Changes diff --git a/README.md b/README.md index 464daed..36958f6 100644 --- a/README.md +++ b/README.md @@ -851,6 +851,83 @@ In this case, `Order::MarkAsCompleted.system.call(...)` will be used in, say, co `Operations::Convenience` is an optional module that contains helpers for simpler operation definitions. See module documentation for more details. +### Asynchronous execution (Sidekiq) + +Any operation can be executed asynchronously via Sidekiq. This part of the +framework is optional and must be required explicitly: + +```ruby +require "operations/sidekiq" +``` + +It expects Sidekiq to be available in the host application. `Money` and +GlobalID-able objects are serialized transparently when those libraries are +loaded, but they are not hard dependencies of the gem. + +The simplest way to schedule an operation is `Operations::Sidekiq::Convenience`. +It exposes a `_async` (and `_try_async`) counterpart for every +singleton method that returns a command: + +```ruby +class Rental::Create + extend Operations::Sidekiq::Convenience + + def self.default + @default ||= Operations::Command.new(...) + end +end + +# Enqueues a job that runs `Rental::Create.default.call!(params, **context)` +Rental::Create.default_async.call(params, **context) + +# Same, but `try_call!` instead of `call!` +Rental::Create.default_try_async.call(params, **context) + +# Scheduling / queue options +Rental::Create.default_async.in(5.minutes).set(queue: :slow).call(params, **context) +Rental::Create.default_async.at(1.hour.from_now).call(params, **context) +``` + +Default Sidekiq options, a custom job class, a default delay and a +retries-exhausted callback can be configured per operation: + +```ruby +class Rental::Create + extend Operations::Sidekiq::Convenience[ + queue: :important, + on_retries_exhausted: Rental::NotifyFailure.default + ] + # ... +end +``` + +`Operations::Sidekiq::Command` can also be used directly when you need more +control, and `Operations::Sidekiq::Serializer` / `Operations::Sidekiq::Deserializer` +handle encoding richer argument types (`Time`, `Date`, `BigDecimal`, `Symbol`, +`Range`, `Module`/`Class`, `Dry::Struct`, `Money`, GlobalID-able objects and +symbol-keyed hashes) that Sidekiq does not support out of the box. + +Jobs are executed by `Operations::Sidekiq::Job`. To add application-specific +instrumentation (for example a Sentry scope or a transaction name) subclass it +and override `#instrument`. Naming the subclass explicitly also keeps the +enqueued job class name stable in Redis: + +```ruby +class OperationJob < Operations::Sidekiq::Job + private + + def instrument(container_class) + Sentry.configure_scope do |scope| + scope.set_transaction_name("Sidekiq/OperationJob/#{container_class}") + yield + end + end +end + +# Point operations at your job class through the convenience configuration: +extend Operations::Sidekiq::Convenience[job_class: OperationJob] +``` + ### Form objects Form objects were refactored to be separate from Command. Please check [UPGRADING_FORMS.md](UPGRADING_FORMS.md) for more details. diff --git a/lib/operations/sidekiq.rb b/lib/operations/sidekiq.rb new file mode 100644 index 0000000..c9e762f --- /dev/null +++ b/lib/operations/sidekiq.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require "operations" +require "sidekiq" + +# A parent module for Operations::Sidekiq::Convenience, +# defines constants and helpers used to run operations +# asynchronously via Sidekiq. +# +# This part of the framework is optional and must be required +# explicitly: +# +# require "operations/sidekiq" +# +# It expects Sidekiq to be available. Serialization of `Money` +# and GlobalID-able objects is supported when those libraries +# are loaded but they are not hard dependencies. +# +# @see Operations::Sidekiq::Convenience +module Operations::Sidekiq + SIDEKIQ_TYPE_KEY = "$sidekiq_type" + SIDEKIQ_VALUE_KEY = "$sidekiq_value" + SIDEKIQ_SYMBOL_KEYS = "$sidekiq_symbol_keys" + + def self.operation_container(operation) + container_class = operation.respond_to?(:operation) ? operation.operation.class : operation.class + + raise "Unable to perform async anonymous operation #{container_class}" unless container_class.name + + container_method = container_class.singleton_methods(false).find do |name| + container_class.send(name) == operation if container_class.method(name).arity.zero? + end + + raise "Unable to find the appropriate command for the operation" unless container_method + + [container_class, container_method] + end +end + +require "operations/sidekiq/serializer" +require "operations/sidekiq/deserializer" +require "operations/sidekiq/job" +require "operations/sidekiq/command" +require "operations/sidekiq/convenience" diff --git a/lib/operations/sidekiq/command.rb b/lib/operations/sidekiq/command.rb new file mode 100644 index 0000000..968c4dd --- /dev/null +++ b/lib/operations/sidekiq/command.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +# Implements a generic Sidekiq wrapper for any operation. +# +# @example +# +# Operations::Sidekiq::Command.new(Rental::Create.default, try_call: false).call(params, **context) +# +class Operations::Sidekiq::Command + extend Dry::Initializer + include Dry::Equalizer(:operation_class_name, :operation_method_name, :try_call, :delay, :time, :sidekiq_options) + include Dry::Monads[:result] + + param :operation_class_name + param :operation_method_name, Operations::Types::Coercible::String + option :try_call, Operations::Types::Bool + option :delay, Operations::Types::Integer.optional, optional: true + option :time, Operations::Types::Time.optional, optional: true + option :sidekiq_options, Operations::Types::Hash, optional: true, default: proc { {} } + option :job_class, optional: true, default: proc { Operations::Sidekiq::Job } + + def in(delay) + self.class.new( + operation_class_name, operation_method_name, + try_call: try_call, delay: delay, sidekiq_options: sidekiq_options, job_class: job_class + ) + end + + def at(time) + self.class.new( + operation_class_name, operation_method_name, + try_call: try_call, time: time, sidekiq_options: sidekiq_options, job_class: job_class + ) + end + + def set(**sidekiq_options) + self.class.new( + operation_class_name, operation_method_name, + try_call: try_call, delay: delay, time: time, sidekiq_options: sidekiq_options, job_class: job_class + ) + end + + def call(params, **context) + perform_args = [ + operation_class_name, + operation_method_name, + serializer.call(params), + serializer.call(context), + try_call ? "try_call!" : "call!" + ] + + Success(jid: schedule_job(perform_args)) + end + + private + + def schedule_job(perform_args) + job = job_class + job = job.set(**sidekiq_options) if sidekiq_options.present? + + if time + job.perform_at(time, *perform_args) + elsif delay + job.perform_in(delay, *perform_args) + else + job.perform_async(*perform_args) + end + end + + def serializer + @serializer ||= Operations::Sidekiq::Serializer.new + end +end diff --git a/lib/operations/sidekiq/convenience.rb b/lib/operations/sidekiq/convenience.rb new file mode 100644 index 0000000..54e7fba --- /dev/null +++ b/lib/operations/sidekiq/convenience.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +# A short-cut for {Operations::Sidekiq::Command} definition. +# +# @example +# +# class Rental::Create +# extend Operations::Sidekiq::Convenience +# +# def self.default +# @default ||= Operations::Command.new(...) +# end +# end +# +# # This will do `Rental::Create.default.call!(params, **context)` inside the job +# Rental::Create.default_async.call(params, **context) +# Rental::Create.default_async.in(5.minutes).set(queue: :slow).call(params, **context) +# +# # This will do `Rental::Create.default.try_call!(params, **context)` inside the job +# Rental::Create.default_try_async.call(params, **context) +# Rental::Create.default_try_async.in(5.minutes).set(queue: :slow).call(params, **context) +# +module Operations::Sidekiq::Convenience + def self.[](on_retries_exhausted: nil, job_class: nil, delay: nil, **sidekiq_options) + Module.new do + define_singleton_method(:extended) do |base| + base.extend Operations::Sidekiq::Convenience + base.instance_variable_set(:@sidekiq_options, sidekiq_options) + base.instance_variable_set(:@on_retries_exhausted, on_retries_exhausted) + base.instance_variable_set(:@job_class, job_class) + base.instance_variable_set(:@delay, delay) + end + end + end + + def on_retries_exhausted + @on_retries_exhausted + end + + def method_missing(name, ...) + name_without_suffix = name.to_s.delete_suffix("_async").delete_suffix("_try").to_sym + + if name.to_s.end_with?("_async") && respond_to?(name_without_suffix) + ivar_name = :"@#{name}" + + if instance_variable_defined?(ivar_name) + instance_variable_get(ivar_name) + else + instance_variable_set(ivar_name, build_sidekiq_command(name, name_without_suffix)) + end + else + super + end + end + + private + + def build_sidekiq_command(name, name_without_suffix) + options = { try_call: name.to_s.end_with?("_try_async"), sidekiq_options: @sidekiq_options || {} } + options[:job_class] = @job_class if @job_class + options[:delay] = @delay if @delay + Operations::Sidekiq::Command.new(self.name, name_without_suffix, **options) + end + + def respond_to_missing?(name, *) + (name.to_s.end_with?("_async") && respond_to?(name.to_s.delete_suffix("_async").delete_suffix("_try").to_sym)) || + super + end +end diff --git a/lib/operations/sidekiq/deserializer.rb b/lib/operations/sidekiq/deserializer.rb new file mode 100644 index 0000000..675f69a --- /dev/null +++ b/lib/operations/sidekiq/deserializer.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +# Used to deserialize operation payload and context +# during the OperationJob processing respectively. +# +# It is the counterpart of {Operations::Sidekiq::Serializer} and +# restores the tagged representation back into the original Ruby +# objects. +class Operations::Sidekiq::Deserializer + DESERIALIZERS = { + "Symbol" => ->(data) { data.to_sym }, + "BigDecimal" => ->(data) { BigDecimal(data) }, + "Date" => ->(data) { data.to_date }, + "Time" => ->(data) { Time.zone.parse(data) }, + "Range" => ->(data) { Range.new(*data) }, + "Module" => ->(data) { data.constantize }, + "Class" => ->(data) { data.constantize }, + "GlobalID" => ->(data) { GlobalID::Locator.locate(data) }, + "Money" => ->(data) { Money.new(*data) }, + "ActiveSupport::TimeWithZone" => ->(data) { Time.zone.parse(data) } + }.freeze + + def call(data) + case data + when Array + data.map { |value| call(value) } + when Hash + if data.key?(Operations::Sidekiq::SIDEKIQ_TYPE_KEY) + deserialize_object( + data[Operations::Sidekiq::SIDEKIQ_TYPE_KEY], + data[Operations::Sidekiq::SIDEKIQ_VALUE_KEY] + ) + else + deserialize_hash(data) + end + else + data + end + end + + private + + def deserialize_hash(hash) + symbol_keys = Set.new(hash.delete(Operations::Sidekiq::SIDEKIQ_SYMBOL_KEYS) || []) + + hash.to_h do |key, value| + [ + symbol_keys.include?(key) ? key.to_sym : key, + call(value) + ] + end + end + + def deserialize_object(type, data) + if type == "ActiveSupport::HashWithIndifferentAccess" + data.transform_values { |value| call(value) }.with_indifferent_access + elsif type == "Dry::Struct" + class_name, attributes = data.first + class_name.constantize.new(**deserialize_hash(attributes).deep_symbolize_keys) + elsif DESERIALIZERS.key?(type) + DESERIALIZERS[type].call(data) + else + raise "Unknown serialized data type #{type} with value #{data}" + end + end +end diff --git a/lib/operations/sidekiq/job.rb b/lib/operations/sidekiq/job.rb new file mode 100644 index 0000000..a023093 --- /dev/null +++ b/lib/operations/sidekiq/job.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +# Executes an operation whenever {Operations::Sidekiq::Command} is used. +# +# We need this generic job in order to avoid creating multiple tiny +# boilerplate job classes. Internally, it uses serialization to +# be able to overcome Sidekiq argument type limitations. +# +# Applications that want to add instrumentation (e.g. a Sentry scope +# or a transaction name) around the operation call can subclass this +# job and override {#instrument}: +# +# class OperationJob < Operations::Sidekiq::Job +# private +# +# def instrument(container_class) +# Sentry.configure_scope do |scope| +# scope.set_transaction_name("Sidekiq/OperationJob/#{container_class}") +# yield +# end +# end +# end +class Operations::Sidekiq::Job + include ::Sidekiq::Job + + sidekiq_options queue: :default + + sidekiq_retries_exhausted do |msg, _exception| + container_class, _, params, context = msg["args"] + container_class = container_class.constantize + next unless container_class.respond_to?(:on_retries_exhausted) && container_class.on_retries_exhausted + + deserializer = Operations::Sidekiq::Deserializer.new + params = deserializer.call(params) + context = deserializer.call(context) + + container_class.on_retries_exhausted.call!(params, **context) + end + + def perform(container_class, container_method, params, context, operation_method = "call!") + deserializer = Operations::Sidekiq::Deserializer.new + operation = container_class.constantize.send(container_method) + params = deserializer.call(params) + context = deserializer.call(context) + + instrument(container_class) do + operation.public_send(operation_method, params, **context) + rescue => e + raise wrap_with_operation_error(container_class, container_method, e) + end + end + + private + + # Hook for wrapping the operation call with application-specific + # instrumentation. The default implementation simply yields. + def instrument(_container_class) + yield + end + + def wrap_with_operation_error(container_class, container_method, err) + parts = "#{container_class}::#{container_method.capitalize}Async".split("::") + operation_error_class = parts.inject(Object) do |obj, part| + obj.const_defined?(part) ? obj.const_get(part) : obj.const_set(part, Module.new) + end + operation_error = operation_error_class.const_set(:Error, Class.new(StandardError)).new(err) + operation_error.tap { |error| error.set_backtrace(err.backtrace) } + end +end diff --git a/lib/operations/sidekiq/serializer.rb b/lib/operations/sidekiq/serializer.rb new file mode 100644 index 0000000..6357d67 --- /dev/null +++ b/lib/operations/sidekiq/serializer.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +# Used to serialize operation payload and context +# during the OperationJob scheduling respectively. +# +# Sidekiq only allows simple JSON types as job arguments. This +# serializer encodes richer types (Time, Date, BigDecimal, Symbol, +# Range, Module/Class, Dry::Struct, Money, GlobalID-able objects and +# symbol-keyed hashes) into a tagged representation that +# {Operations::Sidekiq::Deserializer} can restore losslessly. +class Operations::Sidekiq::Serializer + def call(data) # rubocop:disable Metrics/CyclomaticComplexity + case data + when Array then serialized_array(data) + when ActiveSupport::HashWithIndifferentAccess then serialized_hash_with_indifferent_access(data) + when Hash then serialized_hash(data) + when Time, DateTime then serialized_time(data) + when Date, BigDecimal, Symbol then serialized_as_json(data) + when Range then serialized_range(data) + when Dry::Struct then serialize_dry_struct(data) + when Module, Class then serialized_constant(data) + else serialized_fallback(data) + end + end + + private + + def serialized_fallback(data) + if defined?(Money) && data.is_a?(Money) + serialized_money(data) + elsif data.respond_to?(:to_global_id) + serialized_global_id(data) + else + data + end + end + + def serialized_array(array) + array.map { |value| call(value) } + end + + def serialized_hash_with_indifferent_access(hash) + serialized_object( + "ActiveSupport::HashWithIndifferentAccess", + hash.to_h { |key, value| [key.to_s, call(value)] } + ) + end + + def serialized_hash(hash) + symbol_keys = hash.keys.grep(Symbol).map(&:to_s) + hash.to_h { |key, value| [key.to_s, call(value)] } + .merge(Operations::Sidekiq::SIDEKIQ_SYMBOL_KEYS => symbol_keys) + end + + def serialized_time(data) + serialized_object(data.class.name, data.iso8601(9)) + end + + def serialized_as_json(data) + serialized_object(data.class.name, data.as_json) + end + + def serialized_range(range) + serialized_object(range.class.name, [range.begin, range.end, range.exclude_end?]) + end + + def serialized_money(money) + serialized_object(money.class.name, [money.fractional, money.currency.to_s]) + end + + def serialize_dry_struct(struct) + serialized_object(Dry::Struct.name, serialized_hash(struct.class.name => struct.to_h)) + end + + def serialized_constant(constant) + raise "Unable to serialize an anonymous constant #{constant}" unless constant.name + + serialized_object(constant.class.name, constant.name) + end + + def serialized_global_id(object) + serialized_object("GlobalID", object.to_global_id.to_s) + end + + def serialized_object(key, value) + { + Operations::Sidekiq::SIDEKIQ_TYPE_KEY => key, + Operations::Sidekiq::SIDEKIQ_VALUE_KEY => value + } + end +end diff --git a/operations.gemspec b/operations.gemspec index 8ec16c9..26ade63 100644 --- a/operations.gemspec +++ b/operations.gemspec @@ -29,6 +29,13 @@ Gem::Specification.new do |spec| spec.add_development_dependency "appraisal" spec.add_development_dependency "database_cleaner-active_record" + # Optional integrations exercised by the specs for `operations/sidekiq`. + # These are NOT runtime dependencies of the gem: `operations/sidekiq` must be + # required explicitly and expects the host application to provide Sidekiq + # (and, when used, Money / GlobalID). + spec.add_development_dependency "globalid" + spec.add_development_dependency "money" + spec.add_development_dependency "sidekiq", ">= 6.3" spec.add_development_dependency "sqlite3", ">= 1.4" spec.add_dependency "activerecord", ">= 5.2.0" diff --git a/spec/operations/sidekiq/command_spec.rb b/spec/operations/sidekiq/command_spec.rb new file mode 100644 index 0000000..38dbb6d --- /dev/null +++ b/spec/operations/sidekiq/command_spec.rb @@ -0,0 +1,131 @@ +# frozen_string_literal: true + +RSpec.describe Operations::Sidekiq::Command do + let(:dummy_operation) do + Class.new do + def self.default + @default ||= new + end + + def call!(_params, **_context) + Dry::Monads::Success(foo: 42) + end + alias_method :try_call!, :call! + end + end + let(:command) { described_class.new("DummyOperation", "default", **options) } + let(:options) { { try_call: false } } + let(:serialized_params) { { "name" => "Bruice", "$sidekiq_symbol_keys" => ["name"] } } + let(:serialized_context) { { "context1" => "value1", "$sidekiq_symbol_keys" => ["context1"] } } + + before do + stub_const("DummyOperation", dummy_operation) + allow(DummyOperation.default).to receive(:call!).and_call_original + allow(DummyOperation.default).to receive(:try_call!).and_call_original + end + + describe "#in" do + subject(:in_) { command.in(5.minutes) } + + it "returns a new command with the delay set" do + expect(in_).to be_a(described_class) + expect(in_).not_to eq(command) + expect(in_.delay).to eq(5.minutes) + end + end + + describe "#at" do + subject(:at) { command.at(time) } + + let(:time) { 5.minutes.from_now } + + it "returns a new command with the time set" do + expect(at).to be_a(described_class) + expect(at).not_to eq(command) + expect(at.time).to eq(time) + end + end + + describe "#set" do + subject(:set) { command.set(queue: :slow) } + + it "returns a new command with the sidekiq options set" do + expect(set).to be_a(described_class) + expect(set).not_to eq(command) + expect(set.sidekiq_options).to eq(queue: :slow) + end + end + + describe "#call" do + subject(:call) { command.call({ name: "Bruice" }, context1: "value1") } + + it "schedules the job" do + expect(call).to be_success + expect(Operations::Sidekiq::Job.jobs).to match([ + a_hash_including( + "args" => ["DummyOperation", "default", serialized_params, serialized_context, "call!"], + "queue" => "default" + ) + ]) + end + + it "calls the operation in a job", :inline_jobs do + expect(call).to be_success + expect(DummyOperation.default).to have_received(:call!).with({ name: "Bruice" }, context1: "value1") + end + + context "with try_call set" do + let(:options) { { try_call: true } } + + it "schedules the job with try_call!" do + expect(call).to be_success + expect(Operations::Sidekiq::Job.jobs).to match([ + a_hash_including("args" => ["DummyOperation", "default", serialized_params, serialized_context, "try_call!"]) + ]) + end + + it "calls the operation in a job", :inline_jobs do + expect(call).to be_success + expect(DummyOperation.default).to have_received(:try_call!).with({ name: "Bruice" }, context1: "value1") + end + end + + context "with delay set" do + let(:options) { { try_call: false, delay: 5.minutes } } + + it "schedules the job in the future" do + expect(call).to be_success + expect(Operations::Sidekiq::Job.jobs).to match([ + a_hash_including( + "args" => ["DummyOperation", "default", serialized_params, serialized_context, "call!"], + "at" => be_a(Float) + ) + ]) + end + end + + context "with time set" do + let(:options) { { try_call: false, time: time } } + let(:time) { 5.minutes.from_now } + + it "schedules the job at the given time" do + expect(call).to be_success + expect(Operations::Sidekiq::Job.jobs).to match([ + a_hash_including( + "args" => ["DummyOperation", "default", serialized_params, serialized_context, "call!"], + "at" => time.to_f + ) + ]) + end + end + + context "with sidekiq_options set" do + let(:options) { { try_call: false, sidekiq_options: { queue: :slow } } } + + it "schedules the job on the given queue" do + expect(call).to be_success + expect(Operations::Sidekiq::Job.jobs).to match([a_hash_including("queue" => "slow")]) + end + end + end +end diff --git a/spec/operations/sidekiq/convenience_spec.rb b/spec/operations/sidekiq/convenience_spec.rb new file mode 100644 index 0000000..2987e16 --- /dev/null +++ b/spec/operations/sidekiq/convenience_spec.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true + +RSpec.describe Operations::Sidekiq::Convenience do + let(:dummy_operation) do + Class.new do + extend Operations::Sidekiq::Convenience + + def self.default + @default ||= Operations::Command.new( + new, + contract: Class.new(Operations::Contract) { schema { required(:name).filled(:string) } }.new, + policy: nil + ) + end + + def call(_params, **_context) + Dry::Monads::Success(foo: 42) + end + end + end + let(:serialized_params) { { "name" => "Bruice", "$sidekiq_symbol_keys" => ["name"] } } + let(:serialized_context) { { "context1" => "value1", "$sidekiq_symbol_keys" => ["context1"] } } + + describe ".default_async" do + subject(:call) { dummy_operation.default_async.call({ name: "Bruice" }, context1: "value1") } + + before do + stub_const("DummyOperation", dummy_operation) + allow(dummy_operation.default).to receive(:call!).and_call_original + end + + it "memoizes the async command" do + expect(dummy_operation.default_async).to equal(dummy_operation.default_async) + end + + it "schedules the job" do + expect(call).to be_success + expect(Operations::Sidekiq::Job.jobs).to match([ + a_hash_including( + "args" => ["DummyOperation", "default", serialized_params, serialized_context, "call!"], + "queue" => "default" + ) + ]) + end + + it "calls the operation in a job", :inline_jobs do + expect(call).to be_success + expect(dummy_operation.default).to have_received(:call!).with({ name: "Bruice" }, context1: "value1") + end + end + + describe ".default_try_async" do + subject(:call) { dummy_operation.default_try_async.call({ name: "Bruice" }, context1: "value1") } + + let(:dummy_operation) do + Class.new do + extend Operations::Sidekiq::Convenience[queue: :important] + + def self.default + @default ||= Operations::Command.new( + new, + contract: Class.new(Operations::Contract) { schema { required(:name).filled(:string) } }.new, + policy: nil + ) + end + + def call(_params, **_context) + Dry::Monads::Success(foo: 42) + end + end + end + + before do + stub_const("DummyOperation", dummy_operation) + allow(dummy_operation.default).to receive(:try_call!).and_call_original + end + + it "memoizes the async command" do + expect(dummy_operation.default_try_async).to equal(dummy_operation.default_try_async) + end + + it "schedules the job on the configured queue with try_call!" do + expect(call).to be_success + expect(Operations::Sidekiq::Job.jobs).to match([ + a_hash_including( + "args" => ["DummyOperation", "default", serialized_params, serialized_context, "try_call!"], + "queue" => "important" + ) + ]) + end + + it "calls the operation in a job", :inline_jobs do + expect(call).to be_success + expect(dummy_operation.default).to have_received(:try_call!).with({ name: "Bruice" }, context1: "value1") + end + end + + describe ".on_retries_exhausted" do + subject(:on_retries_exhausted) { dummy_operation.on_retries_exhausted } + + let(:callback) { instance_double(Operations::Command) } + let(:dummy_operation) do + callback_instance = callback + Class.new do + extend Operations::Sidekiq::Convenience[retry: 3, on_retries_exhausted: callback_instance] + + def self.default + @default ||= Operations::Command.new( + new, + contract: Class.new(Operations::Contract) { schema { required(:name).filled(:string) } }.new, + policy: nil + ) + end + + def call(_params, **_context) + Dry::Monads::Success(foo: 42) + end + end + end + + before do + stub_const("DummyOperation", dummy_operation) + allow(callback).to receive(:call!) + end + + it { is_expected.to eq(callback) } + + it "calls the callback when retries are exhausted" do + msg = { + "args" => ["DummyOperation", "default", serialized_params, serialized_context, "call!"] + } + + Operations::Sidekiq::Job.sidekiq_retries_exhausted_block.call(msg, StandardError.new("test")) + + expect(callback).to have_received(:call!).with({ name: "Bruice" }, context1: "value1") + end + end +end diff --git a/spec/operations/sidekiq/deserializer_spec.rb b/spec/operations/sidekiq/deserializer_spec.rb new file mode 100644 index 0000000..1256749 --- /dev/null +++ b/spec/operations/sidekiq/deserializer_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +RSpec.describe Operations::Sidekiq::Deserializer do + subject(:deserializer) { described_class.new } + + before do + struct_class = Class.new(Dry::Struct) do + attribute :percentage, Operations::Types::Any + attribute :fixed_amount, Operations::Types::Any + end + stub_const("PricingPlan", struct_class) + end + + describe "#call" do + it "reverses the serializer for scalars, collections and rich types" do + data = { + number: 42, + "string" => "foobar", + "array" => [4.2, BigDecimal("4.2"), Date.new(2026, 7, 24)], + money: Money.new(42_00, "EUR"), + "pricing_plan" => PricingPlan.new(percentage: BigDecimal("1.2"), fixed_amount: Money.new(30, "EUR")), + hash_with_indifferent_access: { + foo: :bar, + time_with_zone: Time.zone.parse("2026-07-24T12:34:56.123456789Z") + }.with_indifferent_access, + "nested_hash" => { + "range" => 1..42, + "module" => Operations::Sidekiq, + "class" => Operations::Command + } + } + serialized = Operations::Sidekiq::Serializer.new.call(data) + + expect(deserializer.call(serialized)).to eq(data) + end + + it "restores symbol keys tracked by the serializer" do + serialized = { "foo" => 1, "bar" => 2, "$sidekiq_symbol_keys" => ["foo"] } + + expect(deserializer.call(serialized)).to eq(foo: 1, "bar" => 2) + end + + it "restores plain Time-tagged values through the time zone" do + serialized = { "$sidekiq_type" => "Time", "$sidekiq_value" => "2026-07-24T12:34:56.123456789Z" } + + expect(deserializer.call(serialized)).to eq(Time.zone.parse("2026-07-24T12:34:56.123456789Z")) + end + + it "restores ActiveSupport::TimeWithZone-tagged values" do + serialized = { + "$sidekiq_type" => "ActiveSupport::TimeWithZone", + "$sidekiq_value" => "2026-07-24T12:34:56.123456789Z" + } + + expect(deserializer.call(serialized)).to eq(Time.zone.parse("2026-07-24T12:34:56.123456789Z")) + end + + it "locates global ids" do + located = Object.new + allow(GlobalID::Locator).to receive(:locate).with("gid://test/Amenity/1").and_return(located) + serialized = { "$sidekiq_type" => "GlobalID", "$sidekiq_value" => "gid://test/Amenity/1" } + + expect(deserializer.call(serialized)).to be(located) + end + + it "raises on unknown serialized types" do + expect { deserializer.call("$sidekiq_type" => "Nope", "$sidekiq_value" => "x") } + .to raise_error(%r{Unknown serialized data type Nope}) + end + end +end diff --git a/spec/operations/sidekiq/serializer_spec.rb b/spec/operations/sidekiq/serializer_spec.rb new file mode 100644 index 0000000..2659d27 --- /dev/null +++ b/spec/operations/sidekiq/serializer_spec.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +RSpec.describe Operations::Sidekiq::Serializer do + subject(:serializer) { described_class.new } + + describe "#call" do + it "passes JSON-native scalars through untouched" do + expect(serializer.call(42)).to eq(42) + expect(serializer.call("foo")).to eq("foo") + expect(serializer.call(4.2)).to eq(4.2) + expect(serializer.call(nil)).to be_nil + end + + it "serializes symbol-keyed hashes tracking the symbol keys" do + expect(serializer.call({ foo: 1, "bar" => 2 })).to eq( + "foo" => 1, "bar" => 2, "$sidekiq_symbol_keys" => ["foo"] + ) + end + + it "serializes HashWithIndifferentAccess" do + expect(serializer.call({ foo: 1 }.with_indifferent_access)).to eq( + "$sidekiq_type" => "ActiveSupport::HashWithIndifferentAccess", + "$sidekiq_value" => { "foo" => 1 } + ) + end + + it "serializes arrays element-wise" do + expect(serializer.call([1, :two, BigDecimal("3.3")])).to eq([ + 1, + { "$sidekiq_type" => "Symbol", "$sidekiq_value" => "two" }, + { "$sidekiq_type" => "BigDecimal", "$sidekiq_value" => "3.3" } + ]) + end + + it "serializes symbols, big decimals and dates via as_json" do + expect(serializer.call(:foo)).to eq("$sidekiq_type" => "Symbol", "$sidekiq_value" => "foo") + expect(serializer.call(BigDecimal("4.2"))).to eq("$sidekiq_type" => "BigDecimal", "$sidekiq_value" => "4.2") + expect(serializer.call(Date.new(2026, 7, 24))).to eq("$sidekiq_type" => "Date", "$sidekiq_value" => "2026-07-24") + end + + it "serializes times with nanosecond precision" do + time = Time.zone.parse("2026-07-24T12:34:56.123456789Z") + + expect(serializer.call(time)).to eq( + "$sidekiq_type" => "ActiveSupport::TimeWithZone", + "$sidekiq_value" => "2026-07-24T12:34:56.123456789Z" + ) + end + + it "serializes ranges" do + expect(serializer.call(1..42)).to eq("$sidekiq_type" => "Range", "$sidekiq_value" => [1, 42, false]) + end + + it "serializes money" do + expect(serializer.call(Money.new(42_00, "EUR"))).to eq( + "$sidekiq_type" => "Money", "$sidekiq_value" => [42_00, "EUR"] + ) + end + + it "serializes modules and classes by name" do + expect(serializer.call(Operations::Sidekiq)).to eq( + "$sidekiq_type" => "Module", "$sidekiq_value" => "Operations::Sidekiq" + ) + expect(serializer.call(Operations::Command)).to eq( + "$sidekiq_type" => "Class", "$sidekiq_value" => "Operations::Command" + ) + end + + it "raises for anonymous constants" do + expect { serializer.call(Class.new) }.to raise_error(%r{anonymous constant}) + end + + it "serializes dry-structs preserving the class name and nested types" do + stub_const("PricingPlan", Class.new(Dry::Struct) do + attribute :percentage, Operations::Types::Any + attribute :fixed_amount, Operations::Types::Any + end) + struct = PricingPlan.new(percentage: BigDecimal("1.2"), fixed_amount: Money.new(30, "EUR")) + + expect(serializer.call(struct)).to eq( + "$sidekiq_type" => "Dry::Struct", + "$sidekiq_value" => { + "PricingPlan" => { + "percentage" => { "$sidekiq_type" => "BigDecimal", "$sidekiq_value" => "1.2" }, + "fixed_amount" => { "$sidekiq_type" => "Money", "$sidekiq_value" => [30, "EUR"] }, + "$sidekiq_symbol_keys" => %w[percentage fixed_amount] + }, + "$sidekiq_symbol_keys" => [] + } + ) + end + + it "serializes global-id-able objects" do + globalidable = Class.new do + def to_global_id + "gid://test/Amenity/1" + end + end.new + + expect(serializer.call(globalidable)).to eq( + "$sidekiq_type" => "GlobalID", "$sidekiq_value" => "gid://test/Amenity/1" + ) + end + end +end diff --git a/spec/operations/sidekiq_spec.rb b/spec/operations/sidekiq_spec.rb new file mode 100644 index 0000000..3cd675c --- /dev/null +++ b/spec/operations/sidekiq_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +RSpec.describe Operations::Sidekiq do + describe ".operation_container" do + subject(:operation_container) { described_class.operation_container(operation) } + + let(:dummy_operation) do + Class.new do + def self.default + @default ||= Operations::Command.new( + new, + contract: Class.new(Operations::Contract) { schema { required(:name).filled(:string) } }.new, + policy: nil + ) + end + + def call(_params, **_context) + Dry::Monads::Success(foo: 42) + end + end + end + let(:operation) { DummyOperation.default } + + before { stub_const("DummyOperation", dummy_operation) } + + it { is_expected.to eq([DummyOperation, :default]) } + + context "when the operation is not wrapped in a command" do + let(:dummy_operation) do + Class.new do + def self.default + @default ||= new + end + end + end + + it { is_expected.to eq([DummyOperation, :default]) } + end + + context "when no singleton method returns the operation" do + let(:dummy_operation) do + Class.new do + def self.default + new + end + end + end + + it "raises" do + expect { operation_container }.to raise_error(%r{Unable to find the appropriate command}) + end + end + + context "when the operation is anonymous" do + let(:operation) { Class.new.new } + + it "raises" do + expect { operation_container }.to raise_error(%r{anonymous operation}) + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f1817fb..38602ef 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,6 +2,12 @@ require "bundler/setup" require "operations" +require "operations/sidekiq" +require "sidekiq/testing" +require "active_support/time" +# Optional serialization integrations exercised by the operations/sidekiq specs. +require "money" +require "globalid" require "pp" require "active_record" require "database_cleaner-active_record" @@ -10,6 +16,9 @@ ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:") ActiveRecord::Base.logger = Logger.new(nil) +Time.zone = "UTC" # rubocop:disable Rails/TimeZoneAssignment +Sidekiq::Testing.fake! + ActiveRecord::Schema.define do create_table :users do |t| t.column :name, :string @@ -50,4 +59,12 @@ class User < ActiveRecord::Base example.run end end + + config.around(:each, :inline_jobs) do |example| + Sidekiq::Testing.inline! { example.run } + end + + config.after do + Sidekiq::Job.clear_all + end end From 23583f7a82da467e194205b49814ec2451f9575a Mon Sep 17 00:00:00 2001 From: Karol Galanciak Date: Fri, 24 Jul 2026 12:09:39 +0200 Subject: [PATCH 2/4] Require Ruby >= 3.2 and trim CI/appraisal matrix The optional operations/sidekiq integration pulls a modern Sidekiq that needs Ruby 3.2+, so specs were red on the sub-3.2 CI rows. Bump required_ruby_version to >= 3.2, drop the Ruby < 3.2 / Rails < 7.1 rows from the CI matrix and Appraisals (keep 3.2/7.1, 3.3/7.2, 3.4/8.0), run the rubocop job on Ruby 3.2, and remove the now-unused appraisal gemfiles. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 6 +----- Appraisals | 4 ++-- CHANGELOG.md | 1 + gemfiles/rails.5.2.gemfile | 15 --------------- gemfiles/rails.6.0.gemfile | 15 --------------- gemfiles/rails.6.1.gemfile | 15 --------------- gemfiles/rails.7.0.gemfile | 15 --------------- operations.gemspec | 2 +- 8 files changed, 5 insertions(+), 68 deletions(-) delete mode 100644 gemfiles/rails.5.2.gemfile delete mode 100644 gemfiles/rails.6.0.gemfile delete mode 100644 gemfiles/rails.6.1.gemfile delete mode 100644 gemfiles/rails.7.0.gemfile diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ffc670f..386c1ba 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,10 +6,6 @@ jobs: fail-fast: false matrix: include: - - { ruby: '2.7', rails: '5.2' } - - { ruby: '2.7', rails: '6.0' } - - { ruby: '3.0', rails: '6.1' } - - { ruby: '3.1', rails: '7.0' } - { ruby: '3.2', rails: '7.1' } - { ruby: '3.3', rails: '7.2' } - { ruby: '3.4', rails: '8.0' } @@ -30,6 +26,6 @@ jobs: - uses: actions/checkout@v2 - uses: ruby/setup-ruby@v1 with: - ruby-version: 2.7 + ruby-version: '3.2' bundler-cache: true - run: bundle exec rubocop diff --git a/Appraisals b/Appraisals index d0607a0..8c3013e 100644 --- a/Appraisals +++ b/Appraisals @@ -1,9 +1,9 @@ # frozen_string_literal: true -%w[5.2 6.0 6.1 7.0 7.1 7.2 8.0].each do |version| +%w[7.1 7.2 8.0].each do |version| appraise "rails.#{version}" do gem "activerecord", "~> #{version}.0" gem "activesupport", "~> #{version}.0" - gem "sqlite3", version > "7.0" ? "~> 2.1" : "~> 1.4" + gem "sqlite3", "~> 2.1" end end diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a781ed..c8d1748 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Changes +- Require Ruby >= 3.2 and drop Rails < 7.1 from the CI / appraisal matrix (the optional `operations/sidekiq` integration relies on a modern Sidekiq). - Changed `Operations::Command::OperationFailed#message` to include detailed error messages. [\#55](https://github.com/BookingSync/operations/pull/55) ([Azdaroth](https://github.com/Azdaroth)) - Rename Operations::Form#model_name parameter to param_key and make it public preserving backwards compatibility. [\#52](https://github.com/BookingSync/operations/pull/52) ([pyromaniac](https://github.com/pyromaniac)) diff --git a/gemfiles/rails.5.2.gemfile b/gemfiles/rails.5.2.gemfile deleted file mode 100644 index 644c6b6..0000000 --- a/gemfiles/rails.5.2.gemfile +++ /dev/null @@ -1,15 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "bookingsync-rubocop", require: false, github: "BookingSync/bookingsync-rubocop", branch: "main" -gem "rspec" -gem "rubocop", require: false -gem "rubocop-performance", require: false -gem "rubocop-rails", require: false -gem "rubocop-rspec", require: false -gem "activerecord", "~> 5.2.0" -gem "activesupport", "~> 5.2.0" -gem "sqlite3", "~> 1.4" - -gemspec path: "../" diff --git a/gemfiles/rails.6.0.gemfile b/gemfiles/rails.6.0.gemfile deleted file mode 100644 index be371f5..0000000 --- a/gemfiles/rails.6.0.gemfile +++ /dev/null @@ -1,15 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "bookingsync-rubocop", require: false, github: "BookingSync/bookingsync-rubocop", branch: "main" -gem "rspec" -gem "rubocop", require: false -gem "rubocop-performance", require: false -gem "rubocop-rails", require: false -gem "rubocop-rspec", require: false -gem "activerecord", "~> 6.0.0" -gem "activesupport", "~> 6.0.0" -gem "sqlite3", "~> 1.4" - -gemspec path: "../" diff --git a/gemfiles/rails.6.1.gemfile b/gemfiles/rails.6.1.gemfile deleted file mode 100644 index 70dfe7b..0000000 --- a/gemfiles/rails.6.1.gemfile +++ /dev/null @@ -1,15 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "bookingsync-rubocop", require: false, github: "BookingSync/bookingsync-rubocop", branch: "main" -gem "rspec" -gem "rubocop", require: false -gem "rubocop-performance", require: false -gem "rubocop-rails", require: false -gem "rubocop-rspec", require: false -gem "activerecord", "~> 6.1.0" -gem "activesupport", "~> 6.1.0" -gem "sqlite3", "~> 1.4" - -gemspec path: "../" diff --git a/gemfiles/rails.7.0.gemfile b/gemfiles/rails.7.0.gemfile deleted file mode 100644 index 299c385..0000000 --- a/gemfiles/rails.7.0.gemfile +++ /dev/null @@ -1,15 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "bookingsync-rubocop", require: false, github: "BookingSync/bookingsync-rubocop", branch: "main" -gem "rspec" -gem "rubocop", require: false -gem "rubocop-performance", require: false -gem "rubocop-rails", require: false -gem "rubocop-rspec", require: false -gem "activerecord", "~> 7.0.0" -gem "activesupport", "~> 7.0.0" -gem "sqlite3", "~> 1.4" - -gemspec path: "../" diff --git a/operations.gemspec b/operations.gemspec index 26ade63..c4d507a 100644 --- a/operations.gemspec +++ b/operations.gemspec @@ -12,7 +12,7 @@ Gem::Specification.new do |spec| spec.description = "Operations framework" spec.homepage = "https://github.com/BookingSync/operations" spec.license = "MIT" - spec.required_ruby_version = Gem::Requirement.new(">= 2.7.0") + spec.required_ruby_version = Gem::Requirement.new(">= 3.2.0") spec.metadata["homepage_uri"] = spec.homepage spec.metadata["source_code_uri"] = "https://github.com/BookingSync/operations" From 4fafa1eab941e355cdd833bb79c0b0d96beeef60 Mon Sep 17 00:00:00 2001 From: Karol Galanciak Date: Fri, 24 Jul 2026 13:04:24 +0200 Subject: [PATCH 3/4] Fix RuboCop: align TargetRubyVersion with Ruby >= 3.2, regenerate todo The gem now requires Ruby >= 3.2, but .rubocop.yml still targeted 2.7, tripping Gemspec/RequiredRubyVersion. Bumping TargetRubyVersion to 3.2 enables the Ruby 3 idiom cops and, together with rubocop/bookingsync-rubocop version drift since the todo was last generated, surfaced pre-existing offenses across the existing lib/spec files (main was already red). Regenerate .rubocop_todo.yml (the repo's own --auto-gen-config mechanism) to re-absorb that pre-existing debt. The new operations/sidekiq code is clean and appears nowhere in the todo. Co-Authored-By: Claude Opus 4.8 (1M context) --- .rubocop.yml | 2 +- .rubocop_todo.yml | 93 +++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 86175c2..6160b4c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -16,6 +16,6 @@ inherit_mode: - Exclude AllCops: - TargetRubyVersion: 2.7 + TargetRubyVersion: 3.2 Exclude: - gemfiles/* diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 4d0f436..0d4e542 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,19 +1,40 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2024-09-27 05:26:10 UTC using RuboCop version 1.65.0. +# on 2026-07-24 11:03:19 UTC using RuboCop version 1.88.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. # Offense count: 3 -# Configuration parameters: EnforcedStyle, AllowedGems, Include. +Gemspec/AttributeAssignment: + Exclude: + - 'operations.gemspec' + +# Offense count: 6 +# Configuration parameters: EnforcedStyle, AllowedGems. # SupportedStyles: Gemfile, gems.rb, gemspec -# Include: **/*.gemspec, **/Gemfile, **/gems.rb Gemspec/DevelopmentDependencies: Exclude: - 'operations.gemspec' +# Offense count: 3 +# This cop supports safe autocorrection (--autocorrect). +Layout/EmptyLinesAfterModuleInclusion: + Exclude: + - 'spec/operations/command_spec.rb' + +# Offense count: 1 +Lint/DuplicateMethods: + Exclude: + - 'spec/operations/components/preconditions_spec.rb' + +# Offense count: 1 +# This cop supports safe autocorrection (--autocorrect). +Lint/RedundantCopDisableDirective: + Exclude: + - 'lib/operations/form/base.rb' + # Offense count: 5 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes. Metrics/AbcSize: @@ -22,20 +43,76 @@ Metrics/AbcSize: # Offense count: 1 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 149 + Max: 137 + +# Offense count: 2 +# Configuration parameters: CountComments, Max, CountAsOne, AllowedMethods, AllowedPatterns. +Metrics/MethodLength: + Exclude: + - 'lib/operations/components/idempotency.rb' + - 'lib/operations/form/base.rb' # Offense count: 1 # Configuration parameters: CountComments, CountAsOne. Metrics/ModuleLength: - Max: 144 + Max: 141 # Offense count: 3 # Configuration parameters: CountKeywordArgs, MaxOptionalParameters. Metrics/ParameterLists: Max: 7 +# Offense count: 2 +# This cop supports safe autocorrection (--autocorrect). +# Configuration parameters: EnforcedStyle, BlockForwardingName. +# SupportedStyles: anonymous, explicit +Naming/BlockForwarding: + Exclude: + - 'lib/operations/convenience.rb' + +# Offense count: 1 +# Configuration parameters: NamePrefix, ForbiddenPrefixes, AllowedMethods, MethodDefinitionMacros, UseSorbetSigs. +# NamePrefix: is_, has_, have_, does_ +# ForbiddenPrefixes: is_, has_, have_, does_ +# AllowedMethods: is_a? +# MethodDefinitionMacros: define_method, define_singleton_method +Naming/PredicatePrefix: + Exclude: + - 'lib/operations/form/base.rb' + +# Offense count: 22 +# This cop supports safe autocorrection (--autocorrect). +# Configuration parameters: AllowOnlyRestArgument, UseAnonymousForwarding, RedundantRestArgumentNames, RedundantKeywordRestArgumentNames, RedundantBlockArgumentNames. +# RedundantRestArgumentNames: args, arguments +# RedundantKeywordRestArgumentNames: kwargs, options, opts +# RedundantBlockArgumentNames: blk, block, proc +Style/ArgumentsForwarding: + Exclude: + - 'lib/operations.rb' + - 'lib/operations/command.rb' + - 'lib/operations/components/base.rb' + - 'lib/operations/convenience.rb' + - 'lib/operations/form.rb' + - 'lib/operations/form/base.rb' + +# Offense count: 1 +# This cop supports unsafe autocorrection (--autocorrect-all). +Style/ArrayIntersect: + Exclude: + - 'lib/operations/result.rb' + +# Offense count: 10 +# This cop supports safe autocorrection (--autocorrect). +# Configuration parameters: EnforcedStyle. +# SupportedStyles: braces, no_braces +Style/HashAsLastArrayItem: + Exclude: + - 'spec/operations/command_spec.rb' + - 'spec/operations/components/operation_spec.rb' + - 'spec/operations/components/policies_spec.rb' + # Offense count: 1 -# Configuration parameters: IgnoreNameless, IgnoreSymbolicNames. -RSpec/VerifiedDoubles: +# This cop supports safe autocorrection (--autocorrect). +Style/RedundantFreeze: Exclude: - - 'spec/operations/form_spec.rb' + - 'lib/operations/form/builder.rb' From 727767eda134dadcd876be84d775269b0c2899ec Mon Sep 17 00:00:00 2001 From: Karol Galanciak Date: Fri, 24 Jul 2026 13:15:56 +0200 Subject: [PATCH 4/4] Fix all RuboCop offenses in code, not the todo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the previous commit, which absorbed the offenses surfaced by the TargetRubyVersion 3.2 bump into .rubocop_todo.yml. Fix them for real instead: - Style/ArgumentsForwarding + Naming/BlockForwarding: adopt anonymous forwarding (..., *, **, &) across command/convenience/form/etc. - Style/HashAsLastArrayItem, Layout/EmptyLinesAfterModuleInclusion in specs. - Style/RedundantFreeze on the frozen regexp literal; Style/ArrayIntersect -> Array#intersect? in Result#errors_with_code?. - Naming/PredicatePrefix: update the stale inline-disable cop name (Naming/PredicateName was renamed) for the intentional has_attribute?. - Lint/DuplicateMethods: scoped inline-disable for two distinct anonymous Class.new blocks in preconditions_spec (rubocop can't tell them apart). - Gemspec/AttributeAssignment: consolidate spec.metadata into a single hash. This also fixes a latent bug: the trailing `spec.metadata = {...}` was reassigning the whole hash and clobbering homepage_uri/source_code_uri/ changelog_uri set just above. Remaining .rubocop_todo.yml entries are pre-existing Metrics thresholds and the gemspec dev-dependency convention — the same debt already deferred on main (now a strict subset; RSpec/VerifiedDoubles is gone). Refactoring the gem's core methods for those metrics is out of scope here. 334 specs green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .rubocop_todo.yml | 79 +------------------ lib/operations.rb | 4 +- lib/operations/command.rb | 4 +- lib/operations/components/base.rb | 4 +- lib/operations/convenience.rb | 8 +- lib/operations/form.rb | 4 +- lib/operations/form/base.rb | 12 +-- lib/operations/form/builder.rb | 2 +- lib/operations/result.rb | 2 +- operations.gemspec | 12 +-- spec/operations/command_spec.rb | 33 +++++--- spec/operations/components/operation_spec.rb | 2 +- spec/operations/components/policies_spec.rb | 24 ++++-- .../components/preconditions_spec.rb | 2 +- 14 files changed, 68 insertions(+), 124 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 0d4e542..070b079 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,16 +1,11 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2026-07-24 11:03:19 UTC using RuboCop version 1.88.2. +# on 2026-07-24 11:15:02 UTC using RuboCop version 1.88.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 3 -Gemspec/AttributeAssignment: - Exclude: - - 'operations.gemspec' - # Offense count: 6 # Configuration parameters: EnforcedStyle, AllowedGems. # SupportedStyles: Gemfile, gems.rb, gemspec @@ -18,23 +13,6 @@ Gemspec/DevelopmentDependencies: Exclude: - 'operations.gemspec' -# Offense count: 3 -# This cop supports safe autocorrection (--autocorrect). -Layout/EmptyLinesAfterModuleInclusion: - Exclude: - - 'spec/operations/command_spec.rb' - -# Offense count: 1 -Lint/DuplicateMethods: - Exclude: - - 'spec/operations/components/preconditions_spec.rb' - -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -Lint/RedundantCopDisableDirective: - Exclude: - - 'lib/operations/form/base.rb' - # Offense count: 5 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes. Metrics/AbcSize: @@ -61,58 +39,3 @@ Metrics/ModuleLength: # Configuration parameters: CountKeywordArgs, MaxOptionalParameters. Metrics/ParameterLists: Max: 7 - -# Offense count: 2 -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: EnforcedStyle, BlockForwardingName. -# SupportedStyles: anonymous, explicit -Naming/BlockForwarding: - Exclude: - - 'lib/operations/convenience.rb' - -# Offense count: 1 -# Configuration parameters: NamePrefix, ForbiddenPrefixes, AllowedMethods, MethodDefinitionMacros, UseSorbetSigs. -# NamePrefix: is_, has_, have_, does_ -# ForbiddenPrefixes: is_, has_, have_, does_ -# AllowedMethods: is_a? -# MethodDefinitionMacros: define_method, define_singleton_method -Naming/PredicatePrefix: - Exclude: - - 'lib/operations/form/base.rb' - -# Offense count: 22 -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: AllowOnlyRestArgument, UseAnonymousForwarding, RedundantRestArgumentNames, RedundantKeywordRestArgumentNames, RedundantBlockArgumentNames. -# RedundantRestArgumentNames: args, arguments -# RedundantKeywordRestArgumentNames: kwargs, options, opts -# RedundantBlockArgumentNames: blk, block, proc -Style/ArgumentsForwarding: - Exclude: - - 'lib/operations.rb' - - 'lib/operations/command.rb' - - 'lib/operations/components/base.rb' - - 'lib/operations/convenience.rb' - - 'lib/operations/form.rb' - - 'lib/operations/form/base.rb' - -# Offense count: 1 -# This cop supports unsafe autocorrection (--autocorrect-all). -Style/ArrayIntersect: - Exclude: - - 'lib/operations/result.rb' - -# Offense count: 10 -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: EnforcedStyle. -# SupportedStyles: braces, no_braces -Style/HashAsLastArrayItem: - Exclude: - - 'spec/operations/command_spec.rb' - - 'spec/operations/components/operation_spec.rb' - - 'spec/operations/components/policies_spec.rb' - -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -Style/RedundantFreeze: - Exclude: - - 'lib/operations/form/builder.rb' diff --git a/lib/operations.rb b/lib/operations.rb index 4253c60..18d7acb 100644 --- a/lib/operations.rb +++ b/lib/operations.rb @@ -36,8 +36,8 @@ class Error < StandardError class << self attr_reader :default_config - def configure(configuration = nil, **options) - @default_config = (configuration || Configuration).new(**options) + def configure(configuration = nil, **) + @default_config = (configuration || Configuration).new(**) end end diff --git a/lib/operations/command.rb b/lib/operations/command.rb index b2abfca..7565848 100644 --- a/lib/operations/command.rb +++ b/lib/operations/command.rb @@ -286,8 +286,8 @@ def possible(params = EMPTY_HASH, **context) # Returns boolean result instead of Operations::Result for validate method. # True on success and false on failure. - def valid?(*args, **kwargs) - validate(*args, **kwargs).success? + def valid?(*, **) + validate(*, **).success? end def to_hash diff --git a/lib/operations/components/base.rb b/lib/operations/components/base.rb index a9f5459..1e3e4b2 100644 --- a/lib/operations/components/base.rb +++ b/lib/operations/components/base.rb @@ -24,10 +24,10 @@ class Operations::Components::Base private - def result(**options) + def result(**) ::Operations::Result.new( component: self.class.name.demodulize.underscore.to_sym, - **options + ** ) end diff --git a/lib/operations/convenience.rb b/lib/operations/convenience.rb index 87eb3cb..7a3e6ca 100644 --- a/lib/operations/convenience.rb +++ b/lib/operations/convenience.rb @@ -64,10 +64,10 @@ def self.extended(mod) mod.extend Dry::Initializer end - def method_missing(name, *args, **kwargs, &block) + def method_missing(name, ...) name_without_suffix = name.to_s.delete_suffix("!").to_sym if name.to_s.end_with?("!") && respond_to?(name_without_suffix) - public_send(name_without_suffix, *args, **kwargs, &block).method(:call!) + public_send(name_without_suffix, ...).method(:call!) else super end @@ -77,10 +77,10 @@ def respond_to_missing?(name, *) (name.to_s.end_with?("!") && respond_to?(name.to_s.delete_suffix("!").to_sym)) || super end - def contract(prefix = nil, from: OperationContract, &block) + def contract(prefix = nil, from: OperationContract, &) contract = Class.new(from) contract.config.messages.namespace = name.underscore - contract.class_eval(&block) + contract.class_eval(&) const_set(:"#{prefix.to_s.camelize}Contract", contract) end diff --git a/lib/operations/form.rb b/lib/operations/form.rb index 67bcf9d..d9af587 100644 --- a/lib/operations/form.rb +++ b/lib/operations/form.rb @@ -58,10 +58,10 @@ def self.inherited(subclass) option :base_class, type: Operations::Types::Class, default: proc { ::Operations::Form::Base } end) - def initialize(command, hydrator: nil, hydrators: [], model_name: nil, **options) + def initialize(command, hydrator: nil, hydrators: [], model_name: nil, **) hydrators.push(hydrator) if hydrator.present? - super(command, hydrators: hydrators, param_key: model_name, **options) + super(command, hydrators: hydrators, param_key: model_name, **) end def build(params = EMPTY_HASH, **context) diff --git a/lib/operations/form/base.rb b/lib/operations/form/base.rb index 5d3ecd0..165daeb 100644 --- a/lib/operations/form/base.rb +++ b/lib/operations/form/base.rb @@ -60,8 +60,8 @@ def self.extended(base) end end - def attribute(name, **options) - attribute = Operations::Form::Attribute.new(name, **options) + def attribute(name, **) + attribute = Operations::Form::Attribute.new(name, **) self.attributes = attributes.merge( attribute.name => attribute @@ -100,7 +100,7 @@ def type_for_attribute(name) self.class.attributes[name.to_sym].model_type end - def has_attribute?(name) # rubocop:disable Naming/PredicateName + def has_attribute?(name) # rubocop:disable Naming/PredicatePrefix self.class.attributes.key?(name.to_sym) end @@ -117,7 +117,7 @@ def assigned_attributes end # For now we gracefully return nil for unknown methods - def method_missing(name, *args, **kwargs) + def method_missing(name, *, **) build_attribute_name = build_attribute_name(name) build_attribute = self.class.attributes[build_attribute_name] plural_build_attribute = self.class.attributes[build_attribute_name.to_s.pluralize.to_sym] @@ -125,9 +125,9 @@ def method_missing(name, *args, **kwargs) if has_attribute?(name) read_attribute(name) elsif build_attribute&.form - build_attribute.form.new(*args, **kwargs) + build_attribute.form.new(*, **) elsif plural_build_attribute&.form - plural_build_attribute.form.new(*args, **kwargs) + plural_build_attribute.form.new(*, **) elsif operation_result operation_result.context[name] end diff --git a/lib/operations/form/builder.rb b/lib/operations/form/builder.rb index 9cdd984..78eae4e 100644 --- a/lib/operations/form/builder.rb +++ b/lib/operations/form/builder.rb @@ -7,7 +7,7 @@ class Operations::Form::Builder extend Dry::Initializer - NESTED_ATTRIBUTES_SUFFIX = %r{_attributes\z}.freeze + NESTED_ATTRIBUTES_SUFFIX = %r{_attributes\z} option :base_class, Operations::Types::Instance(Class) diff --git a/lib/operations/result.rb b/lib/operations/result.rb index 7b7bb74..efb3d3a 100644 --- a/lib/operations/result.rb +++ b/lib/operations/result.rb @@ -100,7 +100,7 @@ def to_hash(include_command: false) def errors_with_code?(name, *names) names = [name] + names - (errors.map { |error| error.meta[:code] } & names).present? + errors.map { |error| error.meta[:code] }.intersect?(names) end def context_to_hash diff --git a/operations.gemspec b/operations.gemspec index c4d507a..f40ae01 100644 --- a/operations.gemspec +++ b/operations.gemspec @@ -14,9 +14,12 @@ Gem::Specification.new do |spec| spec.license = "MIT" spec.required_ruby_version = Gem::Requirement.new(">= 3.2.0") - spec.metadata["homepage_uri"] = spec.homepage - spec.metadata["source_code_uri"] = "https://github.com/BookingSync/operations" - spec.metadata["changelog_uri"] = "https://github.com/BookingSync/operations" + spec.metadata = { + "homepage_uri" => spec.homepage, + "source_code_uri" => "https://github.com/BookingSync/operations", + "changelog_uri" => "https://github.com/BookingSync/operations", + "rubygems_mfa_required" => "true" + } # Specify which files should be added to the gem when it is released. # The `git ls-files -z` loads the files in the RubyGem that have been added into git. @@ -44,7 +47,4 @@ Gem::Specification.new do |spec| spec.add_dependency "dry-monads" spec.add_dependency "dry-struct" spec.add_dependency "dry-validation" - spec.metadata = { - "rubygems_mfa_required" => "true" - } end diff --git a/spec/operations/command_spec.rb b/spec/operations/command_spec.rb index d12c183..378bc53 100644 --- a/spec/operations/command_spec.rb +++ b/spec/operations/command_spec.rb @@ -54,6 +54,7 @@ let(:operation_class) do Class.new do extend Dry::Initializer + option :repo def call; end @@ -64,6 +65,7 @@ def call; end end) const_set(:Policy, Class.new do extend Dry::Initializer + option :repo def call; end @@ -88,6 +90,7 @@ def call; end before do operation_class.const_set(:Precondition, Class.new do extend Dry::Initializer + option :repo def call; end @@ -242,8 +245,10 @@ def build(**kwargs) errors: have_attributes( to_h: { nil => [ - text: "Unauthorized", - code: :unauthorized + { + text: "Unauthorized", + code: :unauthorized + } ] } ) @@ -269,8 +274,10 @@ def build(**kwargs) errors: have_attributes( to_h: { nil => [ - text: "Unauthorized", - code: :unauthorized + { + text: "Unauthorized", + code: :unauthorized + } ] } ) @@ -663,8 +670,10 @@ def build(**kwargs) errors: have_attributes( to_h: { nil => [ - text: "Unauthorized", - code: :unauthorized + { + text: "Unauthorized", + code: :unauthorized + } ] } ) @@ -749,8 +758,10 @@ def build(**kwargs) errors: have_attributes( to_h: { nil => [ - text: "Unauthorized", - code: :unauthorized + { + text: "Unauthorized", + code: :unauthorized + } ] } ) @@ -828,8 +839,10 @@ def build(**kwargs) errors: have_attributes( to_h: { nil => [ - text: "Unauthorized", - code: :unauthorized + { + text: "Unauthorized", + code: :unauthorized + } ] } ) diff --git a/spec/operations/components/operation_spec.rb b/spec/operations/components/operation_spec.rb index 556451b..71c0048 100644 --- a/spec/operations/components/operation_spec.rb +++ b/spec/operations/components/operation_spec.rb @@ -35,7 +35,7 @@ def call(params, entity:, **) let(:context) { { subject: 42, entity: "Entity" } } context "when operation returns a failure" do - let(:operation) { ->(**) { Dry::Monads::Failure([error: :failure]) } } + let(:operation) { ->(**) { Dry::Monads::Failure([{ error: :failure }]) } } it "renders it as an error" do expect(call) diff --git a/spec/operations/components/policies_spec.rb b/spec/operations/components/policies_spec.rb index cfba759..0756e63 100644 --- a/spec/operations/components/policies_spec.rb +++ b/spec/operations/components/policies_spec.rb @@ -53,8 +53,10 @@ errors: have_attributes( to_h: { nil => [ - text: "Unauthorized!", - code: :unauthorized + { + text: "Unauthorized!", + code: :unauthorized + } ] } ) @@ -83,8 +85,10 @@ errors: have_attributes( to_h: { nil => [ - text: "Failure 1", - code: :failure1 + { + text: "Failure 1", + code: :failure1 + } ] } ) @@ -114,8 +118,10 @@ errors: have_attributes( to_h: { nil => [ - text: "Unauthorized!", - code: :unauthorized + { + text: "Unauthorized!", + code: :unauthorized + } ] } ) @@ -138,8 +144,10 @@ errors: have_attributes( to_h: { nil => [ - text: "Failure 1", - code: :failure1 + { + text: "Failure 1", + code: :failure1 + } ] } ) diff --git a/spec/operations/components/preconditions_spec.rb b/spec/operations/components/preconditions_spec.rb index a7e5993..04b3029 100644 --- a/spec/operations/components/preconditions_spec.rb +++ b/spec/operations/components/preconditions_spec.rb @@ -201,7 +201,7 @@ def self.context_key end end, Class.new do - def self.call(subject1:, subject3: nil, **); end + def self.call(subject1:, subject3: nil, **); end # rubocop:disable Lint/DuplicateMethods def self.context_keys %i[subject1 subject4]