From 0d6ab9feb3a87cd42ab098d0a66e20ca43187438 Mon Sep 17 00:00:00 2001 From: Steve Richert Date: Fri, 31 Mar 2017 13:56:27 -0400 Subject: [PATCH 1/5] Allow developers to define #call with arguments for convenience If the "call" instance method accepts arguments, those arguments will be automatically assigned from the provided context, matching on name. This works for both positional and keyword arguments. If an argument is specified but no matching value is provided in the context, an ArgumentError is raised. --- lib/interactor.rb | 30 +++++- spec/interactor_spec.rb | 210 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 239 insertions(+), 1 deletion(-) diff --git a/lib/interactor.rb b/lib/interactor.rb index 2423630..69edce0 100644 --- a/lib/interactor.rb +++ b/lib/interactor.rb @@ -140,7 +140,7 @@ def run # Raises Interactor::Failure if the context is failed. def run! with_hooks do - call + call(*arguments_for_call) context.called!(self) end rescue @@ -163,4 +163,32 @@ def call # Returns nothing. def rollback end + + private + + # Internal: Determine what arguments (if any) should be passed to the "call" + # instance method when invoking an Interactor. The "call" instance method may + # accept any combination of positional and keyword arguments. This method + # will extract values from the context in order to populate those arguments + # based on their names. + # + # Returns an Array of arguments to be applied as an argument list. + def arguments_for_call + positional_arguments, keyword_arguments = [], {} + available_context_keys = context.to_h.keys + + method(:call).parameters.each do |(type, name)| + next unless available_context_keys.include?(name) + + case type + when :req, :opt + positional_arguments << context[name] + when :keyreq, :key + keyword_arguments[name] = context[name] + end + end + + positional_arguments << keyword_arguments if keyword_arguments.any? + positional_arguments + end end diff --git a/spec/interactor_spec.rb b/spec/interactor_spec.rb index 05eefdf..2153bfc 100644 --- a/spec/interactor_spec.rb +++ b/spec/interactor_spec.rb @@ -1,3 +1,213 @@ describe Interactor do include_examples :lint + + describe "#call" do + let(:interactor) { Class.new.send(:include, described_class) } + + context "positional arguments" do + it "accepts required positional arguments" do + interactor.class_eval do + def call(foo) + context.output = foo + end + end + + result = interactor.call(foo: "baz", hello: "world") + + expect(result.output).to eq("baz") + end + + it "accepts optional positional arguments" do + interactor.class_eval do + def call(foo = "bar") + context.output = foo + end + end + + result = interactor.call(foo: "baz", hello: "world") + + expect(result.output).to eq("baz") + end + + it "assigns absent positional arguments" do + interactor.class_eval do + def call(foo = "bar") + context.output = foo + end + end + + result = interactor.call(hello: "world") + + expect(result.output).to eq("bar") + end + + it "raises an error for missing positional arguments" do + interactor.class_eval do + def call(foo) + context.output = foo + end + end + + expect { interactor.call(hello: "world") }.to raise_error(ArgumentError) + end + end + + context "keyword arguments" do + it "accepts required keyword arguments" do + interactor.class_eval do + def call(foo:) + context.output = foo + end + end + + result = interactor.call(foo: "baz", hello: "world") + + expect(result.output).to eq("baz") + end + + it "accepts optional keyword arguments" do + interactor.class_eval do + def call(foo: "bar") + context.output = foo + end + end + + result = interactor.call(foo: "baz", hello: "world") + + expect(result.output).to eq("baz") + end + + it "assigns absent keyword arguments" do + interactor.class_eval do + def call(foo: "bar") + context.output = foo + end + end + + result = interactor.call(hello: "world") + + expect(result.output).to eq("bar") + end + + it "raises an error for missing keyword arguments" do + interactor.class_eval do + def call(foo:) + context.output = foo + end + end + + expect { interactor.call(hello: "world") }.to raise_error(ArgumentError) + end + end + + context "combination arguments" do + it "accepts required positional with required keyword arguments" do + interactor.class_eval do + def call(foo, hello:) + context.output = [foo, hello] + end + end + + result = interactor.call(foo: "baz", hello: "world") + + expect(result.output).to eq(["baz", "world"]) + end + + it "accepts required positional with optional keyword arguments" do + interactor.class_eval do + def call(foo, hello: "there") + context.output = [foo, hello] + end + end + + result = interactor.call(foo: "baz", hello: "world") + + expect(result.output).to eq(["baz", "world"]) + end + + it "accepts required positional and assigns absent keyword arguments" do + interactor.class_eval do + def call(foo, hello: "there") + context.output = [foo, hello] + end + end + + result = interactor.call(foo: "baz") + + expect(result.output).to eq(["baz", "there"]) + end + + it "accepts optional positional with required keyword arguments" do + interactor.class_eval do + def call(foo = "bar", hello:) + context.output = [foo, hello] + end + end + + result = interactor.call(foo: "baz", hello: "world") + + expect(result.output).to eq(["baz", "world"]) + end + + it "accepts optional positional with optional keyword arguments" do + interactor.class_eval do + def call(foo = "bar", hello: "there") + context.output = [foo, hello] + end + end + + result = interactor.call(foo: "baz", hello: "world") + + expect(result.output).to eq(["baz", "world"]) + end + + it "accepts optional positional and assigns absent keyword arguments" do + interactor.class_eval do + def call(foo = "bar", hello: "there") + context.output = [foo, hello] + end + end + + result = interactor.call(foo: "baz") + + expect(result.output).to eq(["baz", "there"]) + end + + it "assigns absent positional and accepts required keyword arguments" do + interactor.class_eval do + def call(foo = "bar", hello:) + context.output = [foo, hello] + end + end + + result = interactor.call(hello: "world") + + expect(result.output).to eq(["bar", "world"]) + end + + it "assigns absent positional and accepts optional keyword arguments" do + interactor.class_eval do + def call(foo = "bar", hello: "there") + context.output = [foo, hello] + end + end + + result = interactor.call(hello: "world") + + expect(result.output).to eq(["bar", "world"]) + end + + it "assigns absent positional and absent keyword arguments" do + interactor.class_eval do + def call(foo = "bar", hello: "there") + context.output = [foo, hello] + end + end + + result = interactor.call + + expect(result.output).to eq(["bar", "there"]) + end + end + end end From 822c2a48618b79620a4d48ef84025c73a9eecaf8 Mon Sep 17 00:00:00 2001 From: Steve Richert Date: Fri, 31 Mar 2017 15:09:29 -0400 Subject: [PATCH 2/5] Target Ruby versions >= 2.1 Required keyword arguments appeared in Ruby 2.1, after the introduction of keyword arguments in Ruby 2.0. --- .rubocop.yml | 2 +- .travis.yml | 2 -- interactor.gemspec | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 8338bf2..0745e89 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,7 +1,7 @@ # This should always correspond to the required Ruby version specified in the # gemspec. AllCops: - TargetRubyVersion: 2.0 + TargetRubyVersion: 2.1 # TODO: What should we do here? Style/FrozenStringLiteralComment: diff --git a/.travis.yml b/.travis.yml index 46b42c6..423fc3e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,7 +8,6 @@ cache: bundler language: ruby matrix: allow_failures: - - rvm: "2.0" - rvm: ruby-head notifications: webhooks: @@ -16,7 +15,6 @@ notifications: urls: - http://buildlight.collectiveidea.com/ rvm: - - "2.0" - "2.1" - "2.2" - "2.3.4" diff --git a/interactor.gemspec b/interactor.gemspec index e6844b0..b226a50 100644 --- a/interactor.gemspec +++ b/interactor.gemspec @@ -17,7 +17,7 @@ Gem::Specification.new do |spec| spec.files = `git ls-files`.split($INPUT_RECORD_SEPARATOR) spec.test_files = spec.files.grep(/^spec/) - spec.required_ruby_version = ">= 2.0" + spec.required_ruby_version = ">= 2.1" spec.add_development_dependency "bundler", "~> 1.9" spec.add_development_dependency "rake", "~> 10.4" From cf6b27359a18f6c8b9c614dfb054c587eaaa07e4 Mon Sep 17 00:00:00 2001 From: Steve Richert Date: Fri, 31 Mar 2017 15:10:40 -0400 Subject: [PATCH 3/5] Address Rubocop concerns This also improves performance of the Interactor#arguments_for_call method by not duplicating the table of data held internally by the context. As a happy side effect, this adds the Interactor::Context#include? method which may be helpful for developers. --- .rubocop.yml | 2 ++ lib/interactor.rb | 14 ++++++-------- lib/interactor/context.rb | 9 +++++++++ spec/interactor/context_spec.rb | 20 ++++++++++++++++++++ 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 0745e89..080edff 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -43,3 +43,5 @@ Style/StringLiterals: EnforcedStyle: double_quotes Style/SymbolArray: Enabled: false +Style/WordArray: + Enabled: false diff --git a/lib/interactor.rb b/lib/interactor.rb index 69edce0..8cf246f 100644 --- a/lib/interactor.rb +++ b/lib/interactor.rb @@ -173,18 +173,16 @@ def rollback # based on their names. # # Returns an Array of arguments to be applied as an argument list. - def arguments_for_call - positional_arguments, keyword_arguments = [], {} - available_context_keys = context.to_h.keys + def arguments_for_call # rubocop:disable Metrics/MethodLength + positional_arguments = [] + keyword_arguments = {} method(:call).parameters.each do |(type, name)| - next unless available_context_keys.include?(name) + next unless context.include?(name) case type - when :req, :opt - positional_arguments << context[name] - when :keyreq, :key - keyword_arguments[name] = context[name] + when :req, :opt then positional_arguments << context[name] + when :keyreq, :key then keyword_arguments[name] = context[name] end end diff --git a/lib/interactor/context.rb b/lib/interactor/context.rb index cb269d2..238b9f2 100644 --- a/lib/interactor/context.rb +++ b/lib/interactor/context.rb @@ -158,6 +158,15 @@ def rollback! @rolled_back = true end + # Public: Check for the presence of a given key in the context. This does + # not check whether the value is truthy, just whether the key is set to any + # value at all. + # + # Returns true if the key is found or false otherwise. + def include?(key) + table.include?(key.to_sym) + end + # Internal: An Array of successfully called Interactor instances invoked # against this Interactor::Context instance. # diff --git a/spec/interactor/context_spec.rb b/spec/interactor/context_spec.rb index eb8b905..f93fd68 100644 --- a/spec/interactor/context_spec.rb +++ b/spec/interactor/context_spec.rb @@ -164,6 +164,26 @@ module Interactor end end + describe "#include?" do + it "returns true if the key is found" do + context = Context.build(foo: "bar") + + expect(context.include?(:foo)).to eq(true) + end + + it "returns true if the symbolized key is found" do + context = Context.build(foo: "bar") + + expect(context.include?("foo")).to eq(true) + end + + it "returns false if the key is not found" do + context = Context.build(foo: "bar") + + expect(context.include?(:hello)).to eq(false) + end + end + describe "#_called" do let(:context) { Context.build } From a0b968811ab60742c7856409954b630ab4f171bf Mon Sep 17 00:00:00 2001 From: Steve Richert Date: Fri, 31 Mar 2017 15:12:26 -0400 Subject: [PATCH 4/5] Refactor Interactor::Context#fail! to be simpler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …by not interacting directly with OpenStruct#modifiable. --- lib/interactor/context.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/interactor/context.rb b/lib/interactor/context.rb index 238b9f2..57f0087 100644 --- a/lib/interactor/context.rb +++ b/lib/interactor/context.rb @@ -121,7 +121,7 @@ def failure? # # Raises Interactor::Failure initialized with the Interactor::Context. def fail!(context = {}) - context.each { |key, value| modifiable[key.to_sym] = value } + context.each { |key, value| self[key] = value } @failure = true raise Failure, self end From 27af274162bbd39c63270c9cf549442c0f2b7e34 Mon Sep 17 00:00:00 2001 From: Steve Richert Date: Fri, 28 Apr 2017 08:45:17 -0400 Subject: [PATCH 5/5] Back out the concept of magically assigning positional call arguments There were edge cases that weren't yet covered by the specs that would fail for combinations of positional and keyword arguments depending on what's available in the context. For example: class MyInteractor include Interactor def call(a, b: "bears") context.output = [a, b] end end MyInteractor.call(b: "beets").output # => [{ b: "beets" }, "bears"] Plus, this simplifies the interface by giving the developer one choice rather than multiple competing choices that achieve the same thing. --- lib/interactor.rb | 18 ++--- spec/interactor_spec.rb | 160 ++-------------------------------------- 2 files changed, 14 insertions(+), 164 deletions(-) diff --git a/lib/interactor.rb b/lib/interactor.rb index 8cf246f..0109494 100644 --- a/lib/interactor.rb +++ b/lib/interactor.rb @@ -166,24 +166,22 @@ def rollback private - # Internal: Determine what arguments (if any) should be passed to the "call" - # instance method when invoking an Interactor. The "call" instance method may - # accept any combination of positional and keyword arguments. This method - # will extract values from the context in order to populate those arguments - # based on their names. + # Internal: Determine what keyword arguments (if any) should be passed to the + # "call" instance method when invoking an Interactor. The "call" instance + # method may accept any number of keyword arguments. This method will extract + # values from the context in order to populate those arguments based on their + # names. # # Returns an Array of arguments to be applied as an argument list. - def arguments_for_call # rubocop:disable Metrics/MethodLength + def arguments_for_call positional_arguments = [] keyword_arguments = {} method(:call).parameters.each do |(type, name)| + next unless type == :keyreq || type == :key next unless context.include?(name) - case type - when :req, :opt then positional_arguments << context[name] - when :keyreq, :key then keyword_arguments[name] = context[name] - end + keyword_arguments[name] = context[name] end positional_arguments << keyword_arguments if keyword_arguments.any? diff --git a/spec/interactor_spec.rb b/spec/interactor_spec.rb index 2153bfc..d85b8db 100644 --- a/spec/interactor_spec.rb +++ b/spec/interactor_spec.rb @@ -4,54 +4,6 @@ describe "#call" do let(:interactor) { Class.new.send(:include, described_class) } - context "positional arguments" do - it "accepts required positional arguments" do - interactor.class_eval do - def call(foo) - context.output = foo - end - end - - result = interactor.call(foo: "baz", hello: "world") - - expect(result.output).to eq("baz") - end - - it "accepts optional positional arguments" do - interactor.class_eval do - def call(foo = "bar") - context.output = foo - end - end - - result = interactor.call(foo: "baz", hello: "world") - - expect(result.output).to eq("baz") - end - - it "assigns absent positional arguments" do - interactor.class_eval do - def call(foo = "bar") - context.output = foo - end - end - - result = interactor.call(hello: "world") - - expect(result.output).to eq("bar") - end - - it "raises an error for missing positional arguments" do - interactor.class_eval do - def call(foo) - context.output = foo - end - end - - expect { interactor.call(hello: "world") }.to raise_error(ArgumentError) - end - end - context "keyword arguments" do it "accepts required keyword arguments" do interactor.class_eval do @@ -60,9 +12,9 @@ def call(foo:) end end - result = interactor.call(foo: "baz", hello: "world") + result = interactor.call(foo: "bar", hello: "world") - expect(result.output).to eq("baz") + expect(result.output).to eq("bar") end it "accepts optional keyword arguments" do @@ -98,115 +50,15 @@ def call(foo:) expect { interactor.call(hello: "world") }.to raise_error(ArgumentError) end - end - - context "combination arguments" do - it "accepts required positional with required keyword arguments" do - interactor.class_eval do - def call(foo, hello:) - context.output = [foo, hello] - end - end - - result = interactor.call(foo: "baz", hello: "world") - - expect(result.output).to eq(["baz", "world"]) - end - - it "accepts required positional with optional keyword arguments" do - interactor.class_eval do - def call(foo, hello: "there") - context.output = [foo, hello] - end - end - - result = interactor.call(foo: "baz", hello: "world") - - expect(result.output).to eq(["baz", "world"]) - end - - it "accepts required positional and assigns absent keyword arguments" do - interactor.class_eval do - def call(foo, hello: "there") - context.output = [foo, hello] - end - end - - result = interactor.call(foo: "baz") - - expect(result.output).to eq(["baz", "there"]) - end - - it "accepts optional positional with required keyword arguments" do - interactor.class_eval do - def call(foo = "bar", hello:) - context.output = [foo, hello] - end - end - - result = interactor.call(foo: "baz", hello: "world") - - expect(result.output).to eq(["baz", "world"]) - end - it "accepts optional positional with optional keyword arguments" do + it "raises an error for call definitions with non-keyword arguments" do interactor.class_eval do - def call(foo = "bar", hello: "there") - context.output = [foo, hello] - end - end - - result = interactor.call(foo: "baz", hello: "world") - - expect(result.output).to eq(["baz", "world"]) - end - - it "accepts optional positional and assigns absent keyword arguments" do - interactor.class_eval do - def call(foo = "bar", hello: "there") - context.output = [foo, hello] - end - end - - result = interactor.call(foo: "baz") - - expect(result.output).to eq(["baz", "there"]) - end - - it "assigns absent positional and accepts required keyword arguments" do - interactor.class_eval do - def call(foo = "bar", hello:) - context.output = [foo, hello] - end - end - - result = interactor.call(hello: "world") - - expect(result.output).to eq(["bar", "world"]) - end - - it "assigns absent positional and accepts optional keyword arguments" do - interactor.class_eval do - def call(foo = "bar", hello: "there") - context.output = [foo, hello] - end - end - - result = interactor.call(hello: "world") - - expect(result.output).to eq(["bar", "world"]) - end - - it "assigns absent positional and absent keyword arguments" do - interactor.class_eval do - def call(foo = "bar", hello: "there") - context.output = [foo, hello] + def call(foo) + context.output = foo end end - result = interactor.call - - expect(result.output).to eq(["bar", "there"]) + expect { interactor.call(foo: "bar") }.to raise_error(ArgumentError) end end end