From 0f87ca3aa98ff358b5f706a7cfce5ca38b6d23a0 Mon Sep 17 00:00:00 2001 From: Joe Ferris Date: Fri, 16 Sep 2011 16:24:16 -0400 Subject: [PATCH] Use objects, not structs --- lib/factory_girl/attribute_list.rb | 6 +-- lib/factory_girl/callback.rb | 8 +++ lib/factory_girl/factory.rb | 4 +- lib/factory_girl/proxy.rb | 18 +++---- lib/factory_girl/proxy/attributes_for.rb | 1 + lib/factory_girl/proxy/build.rb | 1 + lib/factory_girl/proxy/stub.rb | 1 + lib/factory_girl/trait.rb | 2 +- spec/factory_girl/attribute_list_spec.rb | 4 +- spec/factory_girl/callback_spec.rb | 18 +++++++ spec/factory_girl/proxy/create_spec.rb | 16 +++--- spec/factory_girl/proxy_spec.rb | 65 ++++++++++-------------- spec/support/shared_examples/proxy.rb | 2 +- 13 files changed, 78 insertions(+), 68 deletions(-) diff --git a/lib/factory_girl/attribute_list.rb b/lib/factory_girl/attribute_list.rb index 6da6cd2..09fa249 100644 --- a/lib/factory_girl/attribute_list.rb +++ b/lib/factory_girl/attribute_list.rb @@ -18,8 +18,8 @@ module FactoryGirl add_attribute attribute end - def add_callback(name, &block) - @callbacks << Callback.new(name.to_sym, block) + def add_callback(callback) + @callbacks << callback end def each(&block) @@ -31,7 +31,7 @@ module FactoryGirl end def apply_attributes(attributes_to_apply) - attributes_to_apply.callbacks.each { |callback| add_callback(callback.name, &callback.block) } + attributes_to_apply.callbacks.each { |callback| add_callback(callback) } new_attributes = [] attributes_to_apply.each do |attribute| diff --git a/lib/factory_girl/callback.rb b/lib/factory_girl/callback.rb index c217423..5a8566d 100644 --- a/lib/factory_girl/callback.rb +++ b/lib/factory_girl/callback.rb @@ -10,6 +10,14 @@ module FactoryGirl check_name end + def run(instance, proxy) + case block.arity + when 1 then block.call(instance) + when 2 then block.call(instance, proxy) + else block.call + end + end + private def check_name diff --git a/lib/factory_girl/factory.rb b/lib/factory_girl/factory.rb index 566dcc9..adf2a21 100644 --- a/lib/factory_girl/factory.rb +++ b/lib/factory_girl/factory.rb @@ -90,7 +90,7 @@ module FactoryGirl end def add_callback(name, &block) - @attribute_list.add_callback(name, &block) + @attribute_list.add_callback(Callback.new(name, block)) end def attributes @@ -102,7 +102,7 @@ module FactoryGirl def run(proxy_class, overrides) #:nodoc: proxy = proxy_class.new(build_class) - callbacks.each { |callback| proxy.add_callback(callback.name, callback.block) } + callbacks.each { |callback| proxy.add_callback(callback) } overrides = symbolize_keys(overrides) attributes.each do |attribute| diff --git a/lib/factory_girl/proxy.rb b/lib/factory_girl/proxy.rb index cb23f28..ab73048 100644 --- a/lib/factory_girl/proxy.rb +++ b/lib/factory_girl/proxy.rb @@ -4,6 +4,7 @@ module FactoryGirl attr_reader :callbacks def initialize(klass) + @callbacks = {} end def get(attribute) @@ -15,20 +16,15 @@ module FactoryGirl def associate(name, factory, attributes) end - def add_callback(name, block) - @callbacks ||= {} - @callbacks[name] ||= [] - @callbacks[name] << block + def add_callback(callback) + @callbacks[callback.name] ||= [] + @callbacks[callback.name] << callback end def run_callbacks(name) - if @callbacks && @callbacks[name] - @callbacks[name].each do |block| - case block.arity - when 0 then block.call - when 2 then block.call(@instance, self) - else block.call(@instance) - end + if @callbacks[name] + @callbacks[name].each do |callback| + callback.run(@instance, self) end end end diff --git a/lib/factory_girl/proxy/attributes_for.rb b/lib/factory_girl/proxy/attributes_for.rb index a658f2d..e2287c6 100644 --- a/lib/factory_girl/proxy/attributes_for.rb +++ b/lib/factory_girl/proxy/attributes_for.rb @@ -2,6 +2,7 @@ module FactoryGirl class Proxy #:nodoc: class AttributesFor < Proxy #:nodoc: def initialize(klass) + super(klass) @hash = {} @ignored_attributes = {} end diff --git a/lib/factory_girl/proxy/build.rb b/lib/factory_girl/proxy/build.rb index 55d4a10..4ab9e69 100644 --- a/lib/factory_girl/proxy/build.rb +++ b/lib/factory_girl/proxy/build.rb @@ -2,6 +2,7 @@ module FactoryGirl class Proxy #:nodoc: class Build < Proxy #:nodoc: def initialize(klass) + super(klass) @instance = klass.new @ignored_attributes = {} end diff --git a/lib/factory_girl/proxy/stub.rb b/lib/factory_girl/proxy/stub.rb index fffb5fe..5f4fddc 100644 --- a/lib/factory_girl/proxy/stub.rb +++ b/lib/factory_girl/proxy/stub.rb @@ -4,6 +4,7 @@ module FactoryGirl @@next_id = 1000 def initialize(klass) + super(klass) @instance = klass.new @ignored_attributes = {} @instance.id = next_id diff --git a/lib/factory_girl/trait.rb b/lib/factory_girl/trait.rb index f4abaf4..439b036 100644 --- a/lib/factory_girl/trait.rb +++ b/lib/factory_girl/trait.rb @@ -15,7 +15,7 @@ module FactoryGirl end def add_callback(name, &block) - @attribute_list.add_callback(name, &block) + @attribute_list.add_callback(Callback.new(name, block)) end def attributes diff --git a/spec/factory_girl/attribute_list_spec.rb b/spec/factory_girl/attribute_list_spec.rb index 8a8580d..8f77938 100644 --- a/spec/factory_girl/attribute_list_spec.rb +++ b/spec/factory_girl/attribute_list_spec.rb @@ -65,10 +65,10 @@ describe FactoryGirl::AttributeList, "#add_callback" do let(:proxy) { FactoryGirl::Proxy.new(proxy_class) } it "allows for defining adding a callback" do - subject.add_callback(:after_create) { "Called after_create" } + subject.add_callback(FactoryGirl::Callback.new(:after_create, lambda { "Called after_create" })) subject.callbacks.first.name.should == :after_create - subject.callbacks.first.block.call.should == "Called after_create" + subject.callbacks.first.run(nil, nil).should == "Called after_create" end end diff --git a/spec/factory_girl/callback_spec.rb b/spec/factory_girl/callback_spec.rb index 10884d5..3bed90f 100644 --- a/spec/factory_girl/callback_spec.rb +++ b/spec/factory_girl/callback_spec.rb @@ -9,6 +9,24 @@ describe FactoryGirl::Callback do FactoryGirl::Callback.new("after_create", lambda {}).name.should == :after_create end + it "runs its block with no parameters" do + ran_with = nil + FactoryGirl::Callback.new(:after_create, lambda { ran_with = [] }).run(:one, :two) + ran_with.should == [] + end + + it "runs its block with one parameter" do + ran_with = nil + FactoryGirl::Callback.new(:after_create, lambda { |one| ran_with = [one] }).run(:one, :two) + ran_with.should == [:one] + end + + it "runs its block with two parameters" do + ran_with = nil + FactoryGirl::Callback.new(:after_create, lambda { |one, two| ran_with = [one, two] }).run(:one, :two) + ran_with.should == [:one, :two] + end + it "allows valid callback names to be assigned" do FactoryGirl::Callback::VALID_NAMES.each do |callback_name| expect { FactoryGirl::Callback.new(callback_name, lambda {}) }. diff --git a/spec/factory_girl/proxy/create_spec.rb b/spec/factory_girl/proxy/create_spec.rb index 1754ba8..abeec67 100644 --- a/spec/factory_girl/proxy/create_spec.rb +++ b/spec/factory_girl/proxy/create_spec.rb @@ -25,18 +25,16 @@ describe FactoryGirl::Proxy::Create do end context "callback execution order" do - let(:after_build_callback) { stub("after_build callback", :foo => nil) } - let(:after_create_callback) { stub("after_create callback", :foo => nil) } - let(:callback_sequence) { sequence("after_* callbacks") } - it "runs after_build callbacks before after_create callbacks" do - subject.add_callback(:after_build, proc { after_build_callback.foo }) - subject.add_callback(:after_create, proc { after_create_callback.foo }) - - after_build_callback.expects(:foo).once.in_sequence(callback_sequence) - after_create_callback.expects(:foo).once.in_sequence(callback_sequence) + ran = [] + after_create = FactoryGirl::Callback.new(:after_create, lambda { ran << :after_create }) + after_build = FactoryGirl::Callback.new(:after_build, lambda { ran << :after_build }) + subject.add_callback(after_create) + subject.add_callback(after_build) subject.result(nil) + + ran.should == [:after_build, :after_create] end end end diff --git a/spec/factory_girl/proxy_spec.rb b/spec/factory_girl/proxy_spec.rb index 6188780..2e4b9f7 100644 --- a/spec/factory_girl/proxy_spec.rb +++ b/spec/factory_girl/proxy_spec.rb @@ -23,62 +23,49 @@ describe FactoryGirl::Proxy do end describe "when adding callbacks" do - let(:block_1) { proc { "block 1" } } - let(:block_2) { proc { "block 2" } } - it "adds a callback" do - subject.add_callback(:after_create, block_1) - subject.callbacks[:after_create].should be_eql([block_1]) + callback = FactoryGirl::Callback.new(:after_create, lambda {}) + subject.add_callback(callback) + subject.callbacks[:after_create].should == [callback] end it "adds multiple callbacks of the same name" do - subject.add_callback(:after_create, block_1) - subject.add_callback(:after_create, block_2) - subject.callbacks[:after_create].should be_eql([block_1, block_2]) + one = FactoryGirl::Callback.new(:after_create, lambda {}) + two = FactoryGirl::Callback.new(:after_create, lambda {}) + subject.add_callback(one) + subject.add_callback(two) + subject.callbacks[:after_create].should == [one, two] end it "adds multiple callbacks with different names" do - subject.add_callback(:after_create, block_1) - subject.add_callback(:after_build, block_2) - subject.callbacks[:after_create].should be_eql([block_1]) - subject.callbacks[:after_build].should be_eql([block_2]) + after_create = FactoryGirl::Callback.new(:after_create, lambda {}) + after_build = FactoryGirl::Callback.new(:after_build, lambda {}) + subject.add_callback(after_create) + subject.add_callback(after_build) + subject.callbacks[:after_create].should == [after_create] + subject.callbacks[:after_build].should == [after_build] end end describe "when running callbacks" do - let(:object_1_within_callback) { stub("call_in_create", :foo => true) } - let(:object_2_within_callback) { stub("call_in_create", :foo => true) } - it "runs all callbacks with a given name" do - subject.add_callback(:after_create, proc { object_1_within_callback.foo }) - subject.add_callback(:after_create, proc { object_2_within_callback.foo }) + ran = [] + one = FactoryGirl::Callback.new(:after_create, lambda { ran << :one }) + two = FactoryGirl::Callback.new(:after_create, lambda { ran << :two }) + subject.add_callback(one) + subject.add_callback(two) subject.run_callbacks(:after_create) - object_1_within_callback.should have_received(:foo).once - object_2_within_callback.should have_received(:foo).once + ran.should == [:one, :two] end it "only runs callbacks with a given name" do - subject.add_callback(:after_create, proc { object_1_within_callback.foo }) - subject.add_callback(:after_build, proc { object_2_within_callback.foo }) + ran = [] + after_create = FactoryGirl::Callback.new(:after_create, lambda { ran << :after_create }) + after_build = FactoryGirl::Callback.new(:after_build, lambda { ran << :after_build }) + subject.add_callback(after_create) + subject.add_callback(after_build) subject.run_callbacks(:after_create) - object_1_within_callback.should have_received(:foo).once - object_2_within_callback.should have_received(:foo).never - end - - it "passes in the instance if the block takes an argument" do - subject.instance_variable_set("@instance", object_1_within_callback) - subject.add_callback(:after_create, proc {|spy| spy.foo }) - subject.run_callbacks(:after_create) - object_1_within_callback.should have_received(:foo).once - end - - it "passes in the instance and the proxy if the block takes two arguments" do - subject.instance_variable_set("@instance", object_1_within_callback) - proxy_instance = nil - subject.add_callback(:after_create, proc {|spy, proxy| spy.foo; proxy_instance = proxy }) - subject.run_callbacks(:after_create) - object_1_within_callback.should have_received(:foo).once - proxy_instance.should == subject + ran.should == [:after_create] end end end diff --git a/spec/support/shared_examples/proxy.rb b/spec/support/shared_examples/proxy.rb index 2094f66..a9ae8b0 100644 --- a/spec/support/shared_examples/proxy.rb +++ b/spec/support/shared_examples/proxy.rb @@ -114,7 +114,7 @@ shared_examples_for "proxy with callbacks" do |callback_name| let(:callback) { stub("#{callback_name} callback", :foo => nil) } before do - subject.add_callback(callback_name, proc { callback.foo }) + subject.add_callback(FactoryGirl::Callback.new(callback_name, proc { callback.foo })) end it "runs the #{callback_name} callback" do