diff --git a/interactify.gemspec b/interactify.gemspec index 381d7b5..2be3e88 100644 --- a/interactify.gemspec +++ b/interactify.gemspec @@ -32,11 +32,11 @@ Gem::Specification.new do |spec| spec.files = [ "lib/**/*.rb", - "LICENSE.txt", + "LICENSE.txt", "README.md", "CHANGELOG.md" - ].map { |glob| Dir[glob] }.flatten.reject do |f| - File.directory?(f) || + ].map { |glob| Dir[glob] }.flatten.reject do |f| + File.directory?(f) || f.end_with?("_spec.rb") || f.start_with?(*%w[bin/ test/ spec/ features/ .git .circleci appveyor Gemfile]) || File.expand_path(f) == __FILE__ @@ -47,10 +47,10 @@ Gem::Specification.new do |spec| spec.require_paths = ["lib"] spec.add_dependency "activesupport", ">= 6.0.0" + spec.add_dependency "base64" + spec.add_dependency "bigdecimal" spec.add_dependency "interactor" spec.add_dependency "interactor-contracts" - spec.add_dependency "bigdecimal" - spec.add_dependency "base64" spec.add_dependency "mutex_m" spec.metadata["rubygems_mfa_required"] = "true" end diff --git a/lib/interactify/async/job_klass.rb b/lib/interactify/async/job_klass.rb index 10a273e..9b122f3 100644 --- a/lib/interactify/async/job_klass.rb +++ b/lib/interactify/async/job_klass.rb @@ -24,8 +24,8 @@ def async_job_klass def attach_call(async_job_klass) # e.g. SomeInteractor::AsyncWithSuffix.call(foo: 'bar') - async_job_klass.send(:define_singleton_method, :call) do |context| - call!(context) + async_job_klass.send(:define_singleton_method, :call) do |context = {}| + call!(**context) end end @@ -33,12 +33,19 @@ def attach_call!(async_job_klass) this = self # e.g. SomeInteractor::AsyncWithSuffix.call!(foo: 'bar') - async_job_klass.send(:define_singleton_method, :call!) do |context| + async_job_klass.send(:define_singleton_method, :call!) do |context = {}| # e.g. SomeInteractor::JobWithSuffix job_klass = this.container_klass.const_get("Job#{this.klass_suffix}") # e.g. SomeInteractor::JobWithSuffix.perform_async({foo: 'bar'}) - job_klass.perform_async(this.args(context)) + args = this.args(context) + + # Handle empty hash case to avoid issues with double splat operator + if args.empty? + job_klass.perform_async # Call without arguments for empty payload + else + job_klass.perform_async(args) # Pass as a single argument, not using double splat + end end end diff --git a/lib/interactify/async/job_klass_spec.rb b/lib/interactify/async/job_klass_spec.rb index 9ba7f3c..a17b39b 100644 --- a/lib/interactify/async/job_klass_spec.rb +++ b/lib/interactify/async/job_klass_spec.rb @@ -57,6 +57,22 @@ expect(result).to eq("foo" => "bar") end end + + context "with empty context" do + let(:empty_context) { double("EmptyContext", to_h: {}) } + + before do + allow(container_klass).to receive(:respond_to?).with(:expected_keys).and_return(true) + allow(container_klass).to receive(:expected_keys).and_return([]) + allow(container_klass).to receive(:optional_attrs).and_return([]) + end + + it "returns an empty hash" do + result = subject.args(empty_context) + + expect(result).to eq({}) + end + end end describe "private methods" do @@ -76,6 +92,77 @@ expect(async_job_klass.singleton_methods).to include(:call!) end + + context "when calling with empty context" do + let(:job_klass) { class_double("JobClass", perform_async: true) } + let(:empty_context) { double("EmptyContext", to_h: {}) } + + before do + allow(container_klass).to receive(:const_get).with("Job#{klass_suffix}").and_return(job_klass) + allow(subject).to receive(:args).with(empty_context).and_return({}) + subject.send(:attach_call!, async_job_klass) + end + + it "calls perform_async with empty hash correctly" do + async_job_klass.call!(empty_context) + + expect(job_klass).to have_received(:perform_async).with(no_args) + end + end + + context "when calling with empty args" do + let(:job_klass) { class_double("JobClass") } + let(:empty_context) { double("EmptyContext", to_h: {}) } + + before do + allow(container_klass).to receive(:const_get).with("Job#{klass_suffix}").and_return(job_klass) + allow(subject).to receive(:args).with(empty_context).and_return({}) + subject.send(:attach_call!, async_job_klass) + end + + it "calls perform_async with no keyword arguments" do + # This test verifies the actual implementation detail of how empty hashes are passed + expect(job_klass).to receive(:perform_async) + + async_job_klass.call!(empty_context) + end + + it "handles empty hash correctly without double splat operator issues" do + # This test specifically checks for the issue with parameterless interactors + allow(job_klass).to receive(:perform_async) do |**kwargs| + # This will raise an error if kwargs is not properly handled + expect(kwargs).to eq({}) + end + + expect { async_job_klass.call!(empty_context) }.not_to raise_error + end + + it "reproduces the issue with double splat on empty hash" do + # This test demonstrates the issue with parameterless interactors + # In some Ruby versions, **{} can cause issues + + # Mock the implementation to match the actual code + allow(job_klass).to receive(:method_missing) do |method_name, **kwargs| + if method_name == :perform_async + # This would be the actual implementation in Sidekiq + # The issue is that **{} can be problematic + expect(kwargs).to eq({}) + else + super(method_name, **kwargs) + end + end + + # This should not raise an error, but might in some Ruby versions + expect { async_job_klass.call!(empty_context) }.not_to raise_error + end + + it "calls perform_async without arguments when args is empty" do + # This test verifies our fix for the empty hash issue + expect(job_klass).to receive(:perform_async).with(no_args) + + async_job_klass.call!(empty_context) + end + end end describe "#restrict_to_optional_or_keys_from_contract" do diff --git a/lib/interactify/contracts/mismatching_organizer_error.rb b/lib/interactify/contracts/mismatching_organizer_error.rb index cc5b1af..cab1c75 100644 --- a/lib/interactify/contracts/mismatching_organizer_error.rb +++ b/lib/interactify/contracts/mismatching_organizer_error.rb @@ -10,7 +10,7 @@ def initialize(interactor, organizing, organized_klasses) @organizing = organizing @organized_klasses = organized_klasses - super formatted_message + super(formatted_message) end private diff --git a/lib/interactify/contracts/mismatching_promise_error.rb b/lib/interactify/contracts/mismatching_promise_error.rb index 944ad3d..f140554 100644 --- a/lib/interactify/contracts/mismatching_promise_error.rb +++ b/lib/interactify/contracts/mismatching_promise_error.rb @@ -6,7 +6,7 @@ module Interactify module Contracts class MismatchingPromiseError < Contracts::Failure def initialize(interactor, promising, promised_keys) - super <<~MESSAGE.chomp + super(<<~MESSAGE.chomp) #{interactor} does not promise: #{promising.inspect} diff --git a/lib/interactify/dsl/each_chain.rb b/lib/interactify/dsl/each_chain.rb index 0353644..0eedcf3 100644 --- a/lib/interactify/dsl/each_chain.rb +++ b/lib/interactify/dsl/each_chain.rb @@ -107,7 +107,7 @@ def singular_resource_name end def singular_resource_index_name - "#{singular_resource_name}_index".to_sym + :"#{singular_resource_name}_index" end end end diff --git a/lib/interactify/dsl/if_klass.rb b/lib/interactify/dsl/if_klass.rb index de3c099..b5ab27c 100644 --- a/lib/interactify/dsl/if_klass.rb +++ b/lib/interactify/dsl/if_klass.rb @@ -98,13 +98,11 @@ def attach_inspect end end - # rubocop: disable Naming/BlockForwarding def attach_method(name, &block) attach do |klass, _this| klass.define_method(name, &block) end end - # rubocop: enable Naming/BlockForwarding def attach this = if_builder diff --git a/lib/interactify/dsl/if_klass_spec.rb b/lib/interactify/dsl/if_klass_spec.rb index 4c9987f..602b87a 100644 --- a/lib/interactify/dsl/if_klass_spec.rb +++ b/lib/interactify/dsl/if_klass_spec.rb @@ -50,7 +50,7 @@ end context "with a lambda condition" do - let(:condition) { -> { _1.some_condition } } + let(:condition) { lambda(&:some_condition) } it "works" do klass = subject.klass @@ -66,7 +66,7 @@ end context "with an interactified lambda condition" do - let(:condition) { Interactify(-> { _1.some_condition }) } + let(:condition) { Interactify(lambda(&:some_condition)) } it "works" do klass = subject.klass diff --git a/lib/interactify/dsl/unique_klass_name.rb b/lib/interactify/dsl/unique_klass_name.rb index 6896669..823faf0 100644 --- a/lib/interactify/dsl/unique_klass_name.rb +++ b/lib/interactify/dsl/unique_klass_name.rb @@ -27,7 +27,7 @@ def name_with_suffix(namespace, prefix, suffix) end def normalize_prefix(prefix:, camelize:) - normalized = prefix.to_s.gsub(/::/, "__") + normalized = prefix.to_s.gsub("::", "__") return normalized unless camelize normalized.camelize diff --git a/lib/interactify/interactify.each_spec.rb b/lib/interactify/interactify.each_spec.rb index 0a061e7..a6db07b 100644 --- a/lib/interactify/interactify.each_spec.rb +++ b/lib/interactify/interactify.each_spec.rb @@ -48,7 +48,7 @@ def k(klass) file, line = klass.source_location expect(file).to match __FILE__ - expect(line).to be > 0 + expect(line).to be_positive result = klass.call!(things: [1, 2, 3]) expect(result.a).to eq([1, 2, 3]) diff --git a/spec/lib/interactify/async/job_klass_args_spec.rb b/spec/lib/interactify/async/job_klass_args_spec.rb new file mode 100644 index 0000000..d8e8843 --- /dev/null +++ b/spec/lib/interactify/async/job_klass_args_spec.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Interactify::Async::JobKlass do + let(:container_klass) do + Class.new do + include Interactify + + class << self + attr_reader :called_with + + def call!(**context) + @called_with = context + end + + def expected_keys + %i[id name] + end + end + end + end + + let(:klass_suffix) { "WithSuffix" } + let(:job_klass) { described_class.new(container_klass: container_klass, klass_suffix: klass_suffix) } + let(:async_job_klass) { job_klass.async_job_klass } + + before do + # Define the Job class that would normally be created by Interactify + container_klass.const_set("Job#{klass_suffix}", Class.new do + include Sidekiq::Job if Interactify.sidekiq? + + class << self + attr_reader :perform_async_called, :perform_async_args + + def perform_async(*args) + @perform_async_called = true + @perform_async_args = args.empty? ? nil : args.first + end + + def reset! + @perform_async_called = false + @perform_async_args = nil + end + end + end) + end + + after do + # Clean up the dynamically created constants + container_klass.send(:remove_const, "Job#{klass_suffix}") if container_klass.const_defined?("Job#{klass_suffix}") + end + + describe "#async_job_klass" do + it "creates a class that includes Interactor and Interactor::Contracts" do + expect(async_job_klass.included_modules).to include(Interactor) + expect(async_job_klass.included_modules).to include(Interactor::Contracts) + end + + it "attaches call and call! methods" do + expect(async_job_klass).to respond_to(:call) + expect(async_job_klass).to respond_to(:call!) + end + end + + describe "handling of context parameters" do + let(:job_with_suffix) { container_klass.const_get("Job#{klass_suffix}") } + + before do + job_with_suffix.reset! + end + + context "with empty payload" do + it "calls perform_async with no arguments" do + async_job_klass.call! + + expect(job_with_suffix.perform_async_called).to be true + expect(job_with_suffix.perform_async_args).to be_nil + end + end + + context "with payload data" do + it "calls perform_async with the provided arguments" do + async_job_klass.call!(id: 123, name: "test") + + expect(job_with_suffix.perform_async_called).to be true + + actual_args = if job_with_suffix.perform_async_args.is_a?(Hash) + job_with_suffix.perform_async_args.transform_keys(&:to_sym) + else + job_with_suffix.perform_async_args + end + + expect(actual_args).to eq({ id: 123, name: "test" }) + end + + it "restricts arguments to expected keys from contract" do + async_job_klass.call!(id: 123, name: "test", unexpected: "value") + + expect(job_with_suffix.perform_async_called).to be true + + expected_args = { id: 123, name: "test" } + + actual_keys = if job_with_suffix.perform_async_args.is_a?(Hash) + job_with_suffix.perform_async_args.keys.map(&:to_sym) + else + [] + end + + expect(actual_keys).to match_array(expected_args.keys) + expect(actual_keys).not_to include(:unexpected) + end + end + + context "when using call instead of call!" do + it "delegates to call!" do + expect(async_job_klass).to receive(:call!).with(id: 123) + async_job_klass.call(id: 123) + end + end + end +end diff --git a/spec/support/coverage.rb b/spec/support/coverage.rb index dfd810b..90bef15 100644 --- a/spec/support/coverage.rb +++ b/spec/support/coverage.rb @@ -11,7 +11,7 @@ SimpleCov.start do add_filter "/spec/" - add_filter(/_spec\.rb$/) # This line excludes all files ending with _spec.rb + add_filter(/_spec\.rb$/) # This line excludes all files ending with _spec.rb add_group "Sidekiq jobs" do |src_file| src_file.project_filename =~ %r{lib/interactify/async} && src_file.filename !~ /_spec\.rb/