Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Refactor strategies #346

Merged
merged 1 commit into from

1 participant

@joshuaclayton

No description provided.

@joshuaclayton joshuaclayton Refactor Strategies
This changes Strategy from a class to a module and removes inheritance.
It introduces an Evaluation facade to make building strategies easier
89d5e94
@joshuaclayton joshuaclayton merged commit 89d5e94 into master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 13, 2012
  1. @joshuaclayton

    Refactor Strategies

    joshuaclayton authored
    This changes Strategy from a class to a module and removes inheritance.
    It introduces an Evaluation facade to make building strategies easier
This page is out of date. Refresh to see the latest.
View
8 lib/factory_girl.rb
@@ -3,16 +3,22 @@
require 'factory_girl/errors'
require 'factory_girl/factory_runner'
require 'factory_girl/strategy_calculator'
-require 'factory_girl/strategy'
+require "factory_girl/strategy/build"
+require "factory_girl/strategy/create"
+require "factory_girl/strategy/attributes_for"
+require "factory_girl/strategy/stub"
+require "factory_girl/strategy/null"
require 'factory_girl/registry'
require 'factory_girl/null_factory'
require 'factory_girl/null_object'
+require 'factory_girl/evaluation'
require 'factory_girl/factory'
require 'factory_girl/attribute_assigner'
require 'factory_girl/evaluator'
require 'factory_girl/evaluator_class_definer'
require 'factory_girl/attribute'
require 'factory_girl/callback'
+require 'factory_girl/callback_runner'
require 'factory_girl/declaration_list'
require 'factory_girl/declaration'
require 'factory_girl/sequence'
View
20 lib/factory_girl/callback_runner.rb
@@ -0,0 +1,20 @@
+module FactoryGirl
+ class CallbackRunner
+ def initialize(callbacks, evaluator)
+ @callbacks = callbacks
+ @evaluator = evaluator
+ end
+
+ def update(name, result_instance)
+ callbacks_by_name(name).each do |callback|
+ callback.run(result_instance, @evaluator)
+ end
+ end
+
+ private
+
+ def callbacks_by_name(name)
+ @callbacks.select {|callback| callback.name == name }
+ end
+ end
+end
View
23 lib/factory_girl/evaluation.rb
@@ -0,0 +1,23 @@
+require "observer"
+
+module FactoryGirl
+ class Evaluation
+ include Observable
+
+ def initialize(attribute_assigner, to_create)
+ @attribute_assigner = attribute_assigner
+ @to_create = to_create
+ end
+
+ def create(result_instance)
+ @to_create[result_instance]
+ end
+
+ delegate :object, :hash, to: :@attribute_assigner
+
+ def notify(name, result_instance)
+ changed
+ notify_observers(name, result_instance)
+ end
+ end
+end
View
19 lib/factory_girl/evaluator.rb
@@ -3,7 +3,7 @@
module FactoryGirl
class Evaluator
- class_attribute :callbacks, :attribute_lists
+ class_attribute :attribute_lists
def self.attribute_list
AttributeList.new.tap do |list|
@@ -25,8 +25,6 @@ def initialize(build_strategy, overrides = {})
@overrides.each do |name, value|
singleton_class.send :define_method, name, lambda { value }
end
-
- @build_strategy.add_observer(CallbackRunner.new(self.class.callbacks, self))
end
def association(factory_name, overrides = {})
@@ -56,20 +54,5 @@ def method_missing(method_name, *args, &block)
def __overrides
@overrides
end
-
- private
-
- class CallbackRunner
- def initialize(callbacks, evaluator)
- @callbacks = callbacks
- @evaluator = evaluator
- end
-
- def update(name, result_instance)
- @callbacks.select {|callback| callback.name == name }.each do |callback|
- callback.run(result_instance, @evaluator)
- end
- end
- end
end
end
View
5 lib/factory_girl/evaluator_class_definer.rb
@@ -1,8 +1,7 @@
module FactoryGirl
class EvaluatorClassDefiner
- def initialize(attributes, callbacks, parent_class)
+ def initialize(attributes, parent_class)
@parent_class = parent_class
- @callbacks = callbacks
@attributes = attributes
attributes.each do |attribute|
@@ -12,8 +11,6 @@ def initialize(attributes, callbacks, parent_class)
def evaluator_class
@evaluator_class ||= Class.new(@parent_class).tap do |klass|
- klass.callbacks ||= []
- klass.callbacks += @callbacks
klass.attribute_lists ||= []
klass.attribute_lists += [@attributes]
end
View
9 lib/factory_girl/factory.rb
@@ -35,7 +35,10 @@ def run(strategy_class, overrides, &block) #:nodoc:
evaluator = evaluator_class.new(strategy, overrides.symbolize_keys)
attribute_assigner = AttributeAssigner.new(evaluator, &instance_builder)
- strategy.result(attribute_assigner, to_create).tap(&block)
+ evaluation = Evaluation.new(attribute_assigner, to_create)
+ evaluation.add_observer(CallbackRunner.new(callbacks, evaluator))
+
+ strategy.result(evaluation).tap(&block)
end
def human_names
@@ -97,7 +100,7 @@ def class_name #:nodoc:
end
def evaluator_class
- @evaluator_class ||= EvaluatorClassDefiner.new(attributes, callbacks, parent.evaluator_class).evaluator_class
+ @evaluator_class ||= EvaluatorClassDefiner.new(attributes, parent.evaluator_class).evaluator_class
end
def attributes
@@ -110,7 +113,7 @@ def attributes
end
def callbacks
- processing_order.map {|factory| factory.callbacks }.flatten
+ parent.callbacks + processing_order.map {|factory| factory.callbacks }.flatten
end
def constructor
View
33 lib/factory_girl/strategy.rb
@@ -1,33 +0,0 @@
-require "factory_girl/strategy/build"
-require "factory_girl/strategy/create"
-require "factory_girl/strategy/attributes_for"
-require "factory_girl/strategy/stub"
-require "factory_girl/strategy/null"
-require "observer"
-
-module FactoryGirl
- class Strategy #:nodoc:
- include Observable
-
- def association(runner)
- raise NotImplementedError, "Strategies must return an association"
- end
-
- def result(attribute_assigner, to_create)
- raise NotImplementedError, "Strategies must return a result"
- end
-
- def self.ensure_strategy_exists!(strategy)
- unless Strategy.const_defined? strategy.to_s.camelize
- raise ArgumentError, "Unknown strategy: #{strategy}"
- end
- end
-
- private
-
- def run_callbacks(name, result_instance)
- changed
- notify_observers(name, result_instance)
- end
- end
-end
View
8 lib/factory_girl/strategy/attributes_for.rb
@@ -1,12 +1,12 @@
module FactoryGirl
- class Strategy #:nodoc:
- class AttributesFor < Strategy #:nodoc:
+ module Strategy
+ class AttributesFor
def association(runner)
runner.run(Strategy::Null)
end
- def result(attribute_assigner, to_create)
- attribute_assigner.hash
+ def result(evaluation)
+ evaluation.hash
end
end
end
View
10 lib/factory_girl/strategy/build.rb
@@ -1,13 +1,13 @@
module FactoryGirl
- class Strategy #:nodoc:
- class Build < Strategy #:nodoc:
+ module Strategy
+ class Build
def association(runner)
runner.run
end
- def result(attribute_assigner, to_create)
- attribute_assigner.object.tap do |result_instance|
- run_callbacks(:after_build, result_instance)
+ def result(evaluation)
+ evaluation.object.tap do |instance|
+ evaluation.notify(:after_build, instance)
end
end
end
View
16 lib/factory_girl/strategy/create.rb
@@ -1,16 +1,16 @@
module FactoryGirl
- class Strategy #:nodoc:
- class Create < Strategy #:nodoc:
+ module Strategy
+ class Create
def association(runner)
runner.run
end
- def result(attribute_assigner, to_create)
- attribute_assigner.object.tap do |result_instance|
- run_callbacks(:after_build, result_instance)
- run_callbacks(:before_create, result_instance)
- to_create[result_instance]
- run_callbacks(:after_create, result_instance)
+ def result(evaluation)
+ evaluation.object.tap do |instance|
+ evaluation.notify(:after_build, instance)
+ evaluation.notify(:before_create, instance)
+ evaluation.create(instance)
+ evaluation.notify(:after_create, instance)
end
end
end
View
4 lib/factory_girl/strategy/null.rb
@@ -1,10 +1,10 @@
module FactoryGirl
- class Strategy
+ module Strategy
class Null
def association(runner)
end
- def result(attribute_assigner, to_create)
+ def result(evaluation)
end
end
end
View
12 lib/factory_girl/strategy/stub.rb
@@ -1,16 +1,16 @@
module FactoryGirl
- class Strategy
- class Stub < Strategy #:nodoc:
+ module Strategy
+ class Stub
@@next_id = 1000
def association(runner)
runner.run(Strategy::Stub)
end
- def result(attribute_assigner, to_create)
- attribute_assigner.object.tap do |result_instance|
- stub_database_interaction_on_result(result_instance)
- run_callbacks(:after_stub, result_instance)
+ def result(evaluation)
+ evaluation.object.tap do |instance|
+ stub_database_interaction_on_result(instance)
+ evaluation.notify(:after_stub, instance)
end
end
View
2  lib/factory_girl/strategy_calculator.rb
@@ -15,7 +15,7 @@ def strategy
private
def strategy_is_object?
- @name_or_object.is_a?(Class) && @name_or_object.ancestors.include?(::FactoryGirl::Strategy)
+ @name_or_object.is_a?(Class)
end
def strategy_name_to_object
View
14 spec/factory_girl/evaluator_class_definer_spec.rb
@@ -4,10 +4,9 @@
let(:simple_attribute) { stub("simple attribute", name: :simple, to_proc: lambda { 1 }) }
let(:relative_attribute) { stub("relative attribute", name: :relative, to_proc: lambda { simple + 1 }) }
let(:attribute_that_raises_a_second_time) { stub("attribute that would raise without a cache", name: :raises_without_proper_cache, to_proc: lambda { raise "failed" if @run; @run = true; nil }) }
- let(:callbacks) { [stub("callback 1"), stub("callback 2")] }
let(:attributes) { [simple_attribute, relative_attribute, attribute_that_raises_a_second_time] }
- let(:class_definer) { FactoryGirl::EvaluatorClassDefiner.new(attributes, callbacks, FactoryGirl::Evaluator) }
+ let(:class_definer) { FactoryGirl::EvaluatorClassDefiner.new(attributes, FactoryGirl::Evaluator) }
let(:evaluator) { class_definer.evaluator_class.new(stub("build strategy", add_observer: true)) }
it "returns an evaluator when accessing the evaluator class" do
@@ -32,23 +31,14 @@
class_definer.evaluator_class.attribute_lists.should == [attributes]
end
- it "sets callbacks on the evaluator class" do
- class_definer.evaluator_class.callbacks.should == callbacks
- end
-
context "with a custom evaluator as a parent class" do
- let(:child_callbacks) { [stub("child callback 1"), stub("child callback 2")] }
let(:child_attributes) { [stub("child attribute", name: :simple, to_proc: lambda { 1 })] }
- let(:child_definer) { FactoryGirl::EvaluatorClassDefiner.new(child_attributes, child_callbacks, class_definer.evaluator_class) }
+ let(:child_definer) { FactoryGirl::EvaluatorClassDefiner.new(child_attributes, class_definer.evaluator_class) }
subject { child_definer.evaluator_class }
it "bases its attribute lists on itself and its parent evaluator" do
subject.attribute_lists.should == [attributes, child_attributes]
end
-
- it "bases its callbacks on itself and its parent evaluator" do
- subject.callbacks.should == callbacks + child_callbacks
- end
end
end
View
2  spec/factory_girl/factory_spec.rb
@@ -25,7 +25,7 @@
factory.run(FactoryGirl::Strategy::Build, {})
- strategy.should have_received(:result).with(instance_of(FactoryGirl::AttributeAssigner), block)
+ strategy.should have_received(:result).with(instance_of(FactoryGirl::Evaluation))
end
it "returns associations" do
View
10 spec/factory_girl/strategy/attributes_for_spec.rb
@@ -1,18 +1,18 @@
require 'spec_helper'
describe FactoryGirl::Strategy::AttributesFor do
- let(:result) { { name: "John Doe", gender: "Male", admin: false } }
- let(:attribute_assigner) { stub("attribute assigner", hash: result) }
+ let(:result) { { name: "John Doe", gender: "Male", admin: false } }
+ let(:evaluation) { stub("evaluation", hash: result) }
it_should_behave_like "strategy without association support"
- it "returns the hash from the attribute assigner" do
- subject.result(attribute_assigner, lambda {|item| item }).should == result
+ it "returns the hash from the evaluation" do
+ subject.result(evaluation).should == result
end
it "does not run the to_create block" do
expect do
- subject.result(attribute_assigner, lambda {|item| raise "failed" })
+ subject.result(evaluation)
end.to_not raise_error
end
end
View
20 spec/factory_girl/strategy/create_spec.rb
@@ -5,9 +5,21 @@
it_should_behave_like "strategy with callbacks", :after_build, :before_create, :after_create
it "runs a custom create block" do
- block_run = false
- block = lambda {|instance| block_run = true }
- subject.result(stub("assigner", object: stub("result instance")), block)
- block_run.should be_true
+ evaluation_class = Class.new do
+ def initialize
+ @block_run = false
+ end
+
+ attr_reader :block_run
+
+ def create(*instance)
+ @block_run = true
+ end
+ end
+
+ evaluation = evaluation_class.new
+ evaluation.stubs(object: nil, notify: nil)
+ subject.result(evaluation)
+ evaluation.block_run.should be_true
end
end
View
13 spec/factory_girl/strategy/stub_spec.rb
@@ -14,25 +14,24 @@
end.new
end
- let(:assigner) { stub("attribute assigner", object: result_instance) }
- let(:to_create) { lambda {|instance| instance } }
+ let(:evaluation) { stub("evaluation", object: result_instance, notify: true) }
- it { subject.result(assigner, to_create).should_not be_new_record }
- it { subject.result(assigner, to_create).should be_persisted }
+ it { subject.result(evaluation).should_not be_new_record }
+ it { subject.result(evaluation).should be_persisted }
it "assigns created_at" do
- created_at = subject.result(assigner, to_create).created_at
+ created_at = subject.result(evaluation).created_at
created_at.should == Time.now
Timecop.travel(150000)
- subject.result(assigner, to_create).created_at.should == created_at
+ subject.result(evaluation).created_at.should == created_at
end
[:save, :destroy, :connection, :reload, :update_attribute].each do |database_method|
it "raises when attempting to connect to the database by calling #{database_method}" do
expect do
- subject.result(assigner, to_create).send(database_method)
+ subject.result(evaluation).send(database_method)
end.to raise_error(RuntimeError, "stubbed models are not allowed to access the database")
end
end
View
10 spec/factory_girl/strategy_calculator_spec.rb
@@ -8,16 +8,6 @@
end
end
-describe FactoryGirl::StrategyCalculator, "with a non-FactoryGirl::Strategy object" do
- before { define_class "MyAwesomeStrategy" }
-
- let(:strategy) { MyAwesomeStrategy }
-
- it "returns the strategy object" do
- expect { FactoryGirl::StrategyCalculator.new(strategy).strategy }.to raise_error "unrecognized method MyAwesomeStrategy"
- end
-end
-
describe FactoryGirl::StrategyCalculator do
it "returns the correct strategy object for :build" do
FactoryGirl::StrategyCalculator.new(:build).strategy.should == FactoryGirl::Strategy::Build
View
21 spec/factory_girl/strategy_spec.rb
@@ -1,21 +0,0 @@
-require 'spec_helper'
-
-describe FactoryGirl::Strategy do
- it "raises an error when asking for the result" do
- expect { subject.result(stub("assigner"), lambda {|instance| instance }) }.to raise_error(NotImplementedError, "Strategies must return a result")
- end
-
- it "raises an error when asking for the association" do
- expect { subject.association(stub("runner")) }.to raise_error(NotImplementedError, "Strategies must return an association")
- end
-end
-
-describe FactoryGirl::Strategy, ".ensure_strategy_exists!" do
- it "raises when passed a nonexistent strategy" do
- expect { FactoryGirl::Strategy.ensure_strategy_exists!(:nonexistent) }.to raise_error(ArgumentError, "Unknown strategy: nonexistent")
- end
-
- it "doesn't raise when passed a valid strategy" do
- expect { FactoryGirl::Strategy.ensure_strategy_exists!(:create) }.to_not raise_error
- end
-end
View
30 spec/support/shared_examples/strategy.rb
@@ -70,36 +70,22 @@ def association_named(name, overrides)
end
shared_examples_for "strategy with callbacks" do |*callback_names|
- let(:callback_observer) do
- define_class("CallbackObserver") do
- attr_reader :callbacks_called
-
- def initialize
- @callbacks_called = []
- end
-
- def update(callback_name, assigner)
- @callbacks_called << [callback_name, assigner]
- end
- end.new
- end
-
let(:result_instance) do
define_class("ResultInstance") do
attr_accessor :id
end.new
end
- let(:assigner) { stub("attribute assigner", object: result_instance) }
-
- before { subject.add_observer(callback_observer) }
+ let(:evaluation) { stub("evaluation", object: result_instance, notify: true, create: nil) }
- it "runs the callbacks #{callback_names} with the assigner's object" do
- subject.result(assigner, lambda {|instance| instance })
- callback_observer.callbacks_called.should == callback_names.map {|name| [name, assigner.object] }
+ it "runs the callbacks #{callback_names} with the evaluation's object" do
+ subject.result(evaluation)
+ callback_names.each do |name|
+ evaluation.should have_received(:notify).with(name, evaluation.object)
+ end
end
- it "returns the object from the assigner" do
- subject.result(assigner, lambda {|instance| instance }).should == assigner.object
+ it "returns the object from the evaluation" do
+ subject.result(evaluation).should == evaluation.object
end
end
Something went wrong with that request. Please try again.