Permalink
Browse files

Attributes have a to_proc method and are lazily evaluated on an

anonymous class
  • Loading branch information...
1 parent fba404a commit fba6f332fdeaa80ca31aef8f99599a8482a866bc @joshuaclayton joshuaclayton committed Nov 22, 2011
@@ -6,8 +6,6 @@
module FactoryGirl
class Attribute #:nodoc:
- include Comparable
-
attr_reader :name, :ignored
def initialize(name, ignored)
@@ -17,29 +15,25 @@ def initialize(name, ignored)
end
def add_to(proxy)
+ if @ignored
+ proxy.set_ignored(self, to_proc(proxy))
+ else
+ proxy.set(self, to_proc(proxy))
+ end
end
- def association?
- false
+ def to_proc(proxy)
+ lambda { }
end
- def priority
- 1
+ def association?
+ false
end
def alias_for?(attr)
FactoryGirl.aliases_for(attr).include?(name)
end
- def <=>(another)
- return nil unless another.is_a? Attribute
- self.priority <=> another.priority
- end
-
- def ==(another)
- self.object_id == another.object_id
- end
-
private
def ensure_non_attribute_writer!
@@ -50,13 +44,5 @@ def ensure_non_attribute_writer!
"rather than 'f.#{attribute_name} = value'"
end
end
-
- def set_proxy_value(proxy, value)
- if @ignored
- proxy.set_ignored(self, value)
- else
- proxy.set(self, value)
- end
- end
end
end
@@ -9,8 +9,10 @@ def initialize(name, factory, overrides)
@overrides = overrides
end
- def add_to(proxy)
- proxy.set(self, proxy.association(@factory, @overrides))
+ def to_proc(proxy)
+ factory = @factory
+ overrides = @overrides
+ lambda { proxy.association(factory, overrides) }
end
def association?
@@ -6,13 +6,14 @@ def initialize(name, ignored, block)
@block = block
end
- def add_to(proxy)
- value = @block.arity == 1 ? @block.call(proxy) : proxy.instance_exec(&@block)
- if FactoryGirl::Sequence === value
- raise SequenceAbuseError
- end
+ def to_proc(proxy)
+ block = @block
- set_proxy_value(proxy, value)
+ lambda {
+ value = block.arity == 1 ? block.call(proxy) : proxy.instance_exec(&block)
+ raise SequenceAbuseError if FactoryGirl::Sequence === value
+ value
+ }
end
end
end
@@ -7,9 +7,9 @@ def initialize(name, sequence, ignored)
@sequence = sequence
end
- def add_to(proxy)
- value = FactoryGirl.generate(@sequence)
- set_proxy_value(proxy, value)
+ def to_proc(proxy)
+ sequence = @sequence
+ lambda { FactoryGirl.generate(sequence) }
end
end
@@ -1,27 +1,14 @@
module FactoryGirl
class Attribute #:nodoc:
-
class Static < Attribute #:nodoc:
-
- attr_reader :value
-
def initialize(name, value, ignored)
super(name, ignored)
@value = value
end
- def add_to(proxy)
- set_proxy_value(proxy, @value)
- end
-
- def priority
- 0
- end
-
- def ==(another)
- self.name == another.name &&
- self.value == another.value &&
- self.ignored == another.ignored
+ def to_proc(proxy)
+ value = @value
+ lambda { value }
end
end
end
@@ -15,19 +15,16 @@ def define_attribute(attribute)
end
def each(&block)
- sorted_attributes.each(&block)
+ @attributes.each(&block)
end
def apply_attributes(attributes_to_apply)
- new_attributes = []
-
attributes_to_apply.each do |attribute|
new_attribute = find_attribute(attribute.name) || attribute
delete_attribute(attribute.name)
- new_attributes << new_attribute
- end
- prepend_attributes new_attributes
+ add_attribute new_attribute
+ end
end
private
@@ -37,19 +34,6 @@ def add_attribute(attribute)
attribute
end
- def prepend_attributes(new_attributes)
- @attributes.unshift *new_attributes
- end
-
- def sorted_attributes
- attributes_hash = attributes_hash_by_priority
-
- attributes_hash.keys.sort.inject([]) do |result, key|
- result << attributes_hash[key]
- result
- end.flatten
- end
-
def ensure_attribute_not_defined!(attribute)
if attribute_defined?(attribute.name)
raise AttributeDefinitionError, "Attribute already defined: #{attribute.name}"
@@ -75,13 +59,5 @@ def find_attribute(attribute_name)
def delete_attribute(attribute_name)
@attributes.delete_if {|attrib| attrib.name == attribute_name }
end
-
- def attributes_hash_by_priority
- @attributes.inject({}) do |result, attribute|
- result[attribute.priority] ||= []
- result[attribute.priority] << attribute
- result
- end
- end
end
end
View
@@ -7,31 +7,16 @@
module FactoryGirl
class Proxy #:nodoc:
def initialize(klass, callbacks = [])
- @callbacks = callbacks.inject({}) do |result, callback|
- result[callback.name] ||= []
- result[callback.name] << callback
- result
- end
-
- @instance = NullInstanceWrapper.new
- end
-
- def get(attribute)
- @instance.get(attribute)
- end
-
- def set(attribute, value)
- @instance.set(attribute.name, value)
+ @callbacks = process_callbacks(callbacks)
+ @proxy = ObjectWrapper.new(klass)
end
- def set_ignored(attribute, value)
- @instance.set_ignored(attribute.name, value)
- end
+ delegate :get, :set, :set_ignored, :to => :@proxy
def run_callbacks(name)
if @callbacks[name]
@callbacks[name].each do |callback|
- callback.run(@instance.object, self)
+ callback.run(result_instance, self)
end
end
end
@@ -71,10 +56,6 @@ def run_callbacks(name)
def association(name, overrides = {})
end
- def method_missing(method, *args, &block)
- get(method)
- end
-
def result(to_create)
raise NotImplementedError, "Strategies must return a result"
end
@@ -85,39 +66,80 @@ def self.ensure_strategy_exists!(strategy)
end
end
- class InstanceWrapper
- attr_reader :object
- def initialize(object = nil)
- @object = object
- @ignored_attributes = {}
+ private
+
+ def method_missing(method, *args, &block)
+ get(method)
+ end
+
+ def process_callbacks(callbacks)
+ callbacks.inject({}) do |result, callback|
+ result[callback.name] ||= []
+ result[callback.name] << callback
+ result
+ end
+ end
+
+ def result_instance
+ @proxy.object
+ end
+
+ def result_hash
+ @proxy.to_hash
+ end
+
+ class ObjectWrapper
+ def initialize(klass)
+ @klass = klass
+ @attributes = []
+ @cached_attribute_values = {}
+ end
+
+ def to_hash
+ @attributes.inject({}) do |result, attribute|
+ result[attribute] = get(attribute)
+ result
+ end
+ end
+
+ def object
+ @object ||= @klass.new
+ assign_object_attributes
+ @object
+ end
+
+ def set(attribute, value)
+ define_attribute(attribute, value)
+ @attributes << attribute.name
end
def set_ignored(attribute, value)
- @ignored_attributes[attribute] = value
+ define_attribute(attribute, value)
end
def get(attribute)
- if @ignored_attributes.has_key?(attribute)
- @ignored_attributes[attribute]
- else
- get_attr(attribute)
- end
+ @cached_attribute_values[attribute] ||= anonymous_instance.send(attribute)
@metaskills

metaskills Jan 6, 2012

Contributor

What good is sending methods that you expect your model to respond to (deep in @proxy.@object) to the anonymous instance? It has no logic for the attribute. So previous version relying on talking to their models in lazy blocks end up talking to a stupid anonymous class instance with no delegation.

end
- end
- class NullInstanceWrapper < InstanceWrapper
- def get_attr(attribute); end
- def set(attribute, value); end
- end
+ private
- class ClassInstanceWrapper < InstanceWrapper
@metaskills

metaskills Jan 6, 2012

Contributor

I believe this was where you would get messages sent back to your instance.

- def get_attr(attribute); @object.send(attribute); end
- def set(attribute, value); @object.send(:"#{attribute}=", value); end
- end
+ def define_attribute(attribute, value)
+ anonymous_class.send(:define_method, attribute.name, value)
+ end
- class HashInstanceWrapper < InstanceWrapper
- def get_attr(attribute); @object[attribute]; end
- def set(attribute, value); @object[attribute] = value; end
+ def assign_object_attributes
+ (@attributes - @cached_attribute_values.keys).each do |attribute|
+ @object.send("#{attribute}=", get(attribute))
+ end
+ end
+
+ def anonymous_class
+ @anonymous_class ||= Class.new
+ end
+
+ def anonymous_instance
+ @anonymous_instance ||= anonymous_class.new
+ end
end
end
end
@@ -1,18 +1,13 @@
module FactoryGirl
class Proxy #:nodoc:
class AttributesFor < Proxy #:nodoc:
- def initialize(klass, callbacks = [])
- super
- @instance = HashInstanceWrapper.new({})
- end
-
def set(attribute, value)
return if attribute.is_a? Attribute::Association
super
end
def result(to_create)
- @instance.object
+ result_hash
end
end
end
@@ -1,19 +1,14 @@
module FactoryGirl
class Proxy #:nodoc:
class Build < Proxy #:nodoc:
- def initialize(klass, callbacks = [])
- super
- @instance = ClassInstanceWrapper.new(klass.new)
- end
-
def association(factory_name, overrides = {})
factory = FactoryGirl.factory_by_name(factory_name)
factory.run(get_method(overrides[:method]), overrides.except(:method))
end
def result(to_create)
run_callbacks(:after_build)
- @instance.object
+ result_instance
end
private
Oops, something went wrong.

4 comments on commit fba6f33

Contributor

metaskills replied Jan 6, 2012

I have not tracked down the tests that would have failed if they were not deleted or change in this commit, but I have a gut feeling theses tests are so white-box and contrived that a critical error was introduced in 2.3.2. No longer do your lazy attribute blocks allow you to talk to your model. Instead you now get:

NoMethodError: undefined method `killer_instance_method' for #<#<Class:0x10c1afa60>:0x10c1aeea8>
Owner

joshuaclayton replied Jan 6, 2012

@metaskills can you provide a test that passed before 2.3.2 that currently fails on master regarding this?

Remember, you only have access to the resulting model instance in the after_* callbacks. It's always been this way. You've never ever been able to access attributes that existed on the model instance from within FG attributes themselves, so it doesn't actually matter that these methods are on an anonymous class.

Again, an example (instead of a gut feeling) would be great help here.

Owner

joshuaclayton replied Jan 6, 2012

@metaskills looks like I was wrong - I'll take a look to see how this actually worked before and see what I can do. A pull request would be appreciated.

Contributor

metaskills replied Jan 6, 2012

Thanks Josh! I started a proper issue thread here thoughtbot#264

Please sign in to comment.