Skip to content

Commit

Permalink
Attributes have a to_proc method and are lazily evaluated on an
Browse files Browse the repository at this point in the history
anonymous class
  • Loading branch information
joshuaclayton committed Nov 26, 2011
1 parent fba404a commit fba6f33
Show file tree
Hide file tree
Showing 22 changed files with 178 additions and 248 deletions.
32 changes: 9 additions & 23 deletions lib/factory_girl/attribute.rb
Expand Up @@ -6,8 +6,6 @@
module FactoryGirl

class Attribute #:nodoc:
include Comparable

attr_reader :name, :ignored

def initialize(name, ignored)
Expand All @@ -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!
Expand All @@ -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
6 changes: 4 additions & 2 deletions lib/factory_girl/attribute/association.rb
Expand Up @@ -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?
Expand Down
13 changes: 7 additions & 6 deletions lib/factory_girl/attribute/dynamic.rb
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions lib/factory_girl/attribute/sequence.rb
Expand Up @@ -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

Expand Down
19 changes: 3 additions & 16 deletions lib/factory_girl/attribute/static.rb
@@ -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
Expand Down
30 changes: 3 additions & 27 deletions lib/factory_girl/attribute_list.rb
Expand Up @@ -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
Expand All @@ -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}"
Expand All @@ -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
114 changes: 68 additions & 46 deletions lib/factory_girl/proxy.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)

This comment has been minimized.

Copy link
@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

This comment has been minimized.

Copy link
@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
7 changes: 1 addition & 6 deletions lib/factory_girl/proxy/attributes_for.rb
@@ -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
Expand Down
7 changes: 1 addition & 6 deletions lib/factory_girl/proxy/build.rb
@@ -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
Expand Down

4 comments on commit fba6f33

@metaskills
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

@joshuaclayton
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

@joshuaclayton
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

@metaskills
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Please sign in to comment.