Add FactoryGirl::Definition

Both Factory and Trait have similar methods and interact with a
DefinitionProxy. The idea here is to move the interface DefinitionProxy
expects to a separate class and both Factory and Trait can delegate to
an instance of Definition.

lib/factory_girl.rb

@@ -23,6 +23,7 @@
 require 'factory_girl/attribute_list'
 require 'factory_girl/trait'
 require 'factory_girl/aliases'
+require 'factory_girl/definition'
 require 'factory_girl/definition_proxy'
 require 'factory_girl/syntax/methods'
 require 'factory_girl/syntax/default'

lib/factory_girl/attribute_list.rb

@@ -2,6 +2,8 @@ module FactoryGirl
   class AttributeList
     include Enumerable
 
+    attr_reader :declarations
+
     def initialize(name = nil)
       @name         = name
       @attributes   = {}

lib/factory_girl/declaration/implicit.rb

@@ -23,8 +23,7 @@ def build
         elsif FactoryGirl.sequences.registered?(name)
           [Attribute::Sequence.new(name, name, @ignored)]
         else
-          trait_root = @factory || FactoryGirl
-          trait_root.trait_by_name(name).attributes.to_a
+          @factory.trait_by_name(name).attributes.to_a
         end
       end
     end

lib/factory_girl/declaration_list.rb

@@ -1,15 +1,15 @@
 module FactoryGirl
   class DeclarationList
     def initialize
-      @definitions = []
+      @declarations = []
     end
 
     def to_attributes
-      @definitions.inject([]) {|result, definition| result += definition.to_attributes }
+      @declarations.inject([]) {|result, declaration| result += declaration.to_attributes }
     end
 
     def method_missing(name, *args, &block)
-      @definitions.send(name, *args, &block)
+      @declarations.send(name, *args, &block)
     end
   end
 end

lib/factory_girl/definition.rb

@@ -0,0 +1,40 @@
+module FactoryGirl
+  class Definition
+    attr_reader :callbacks, :defined_traits, :attribute_list
+
+    def initialize(name = nil)
+      @attribute_list = AttributeList.new(name)
+      @callbacks      = []
+      @defined_traits = []
+      @to_create      = nil
+    end
+
+    delegate :declare_attribute, :to => :attribute_list
+
+    def add_callback(callback)
+      @callbacks << callback
+    end
+
+    def to_create(&block)
+      if block_given?
+        @to_create = block
+      else
+        @to_create
+      end
+    end
+
+    def define_trait(trait)
+      @defined_traits << trait
+    end
+
+    def trait_by_name(name)
+      trait_for(name) || FactoryGirl.trait_by_name(name)
+    end
+
+    private
+
+    def trait_for(name)
+      defined_traits.detect {|trait| trait.name == name }
+    end
+  end
+end

lib/factory_girl/definition_proxy.rb

@@ -8,8 +8,8 @@ class DefinitionProxy
 
     attr_reader :child_factories
 
-    def initialize(factory, ignore = false)
-      @factory         = factory
+    def initialize(definition, ignore = false)
+      @definition      = definition
       @ignore          = ignore
       @child_factories = []
     end
@@ -41,11 +41,11 @@ def add_attribute(name, value = nil, &block)
         Declaration::Static.new(name, value, @ignore)
       end
 
-      @factory.declare_attribute(declaration)
+      @definition.declare_attribute(declaration)
     end
 
     def ignore(&block)
-      proxy = DefinitionProxy.new(@factory, true)
+      proxy = DefinitionProxy.new(@definition, true)
       proxy.instance_eval(&block)
     end
 
@@ -82,7 +82,7 @@ def ignore(&block)
     # are equivalent.
     def method_missing(name, *args, &block)
       if args.empty? && block.nil?
-        @factory.declare_attribute(Declaration::Implicit.new(name, @factory, @ignore))
+        @definition.declare_attribute(Declaration::Implicit.new(name, @definition, @ignore))
       elsif args.first.is_a?(Hash) && args.first.has_key?(:factory)
         association(name, *args)
       else
@@ -135,31 +135,31 @@ def sequence(name, start_value = 1, &block)
     #    name of the factory. For example, a "user" association will by
     #    default use the "user" factory.
     def association(name, options = {})
-      @factory.declare_attribute(Declaration::Association.new(name, options))
+      @definition.declare_attribute(Declaration::Association.new(name, options))
     end
 
     def after_build(&block)
-      @factory.add_callback(Callback.new(:after_build, block))
+      @definition.add_callback(Callback.new(:after_build, block))
     end
 
     def after_create(&block)
-      @factory.add_callback(Callback.new(:after_create, block))
+      @definition.add_callback(Callback.new(:after_create, block))
     end
 
     def after_stub(&block)
-      @factory.add_callback(Callback.new(:after_stub, block))
+      @definition.add_callback(Callback.new(:after_stub, block))
     end
 
     def to_create(&block)
-      @factory.to_create(&block)
+      @definition.to_create(&block)
     end
 
     def factory(name, options = {}, &block)
       @child_factories << [name, options, block]
     end
 
     def trait(name, &block)
-      @factory.define_trait(Trait.new(name, &block))
+      @definition.define_trait(Trait.new(name, &block))
     end
   end
 end

lib/factory_girl/factory.rb

@@ -3,7 +3,7 @@
 
 module FactoryGirl
   class Factory
-    attr_reader :name #:nodoc:
+    attr_reader :name, :definition #:nodoc:
 
     def initialize(name, options = {}) #:nodoc:
       assert_valid_options(options)
@@ -13,22 +13,17 @@ def initialize(name, options = {}) #:nodoc:
       @traits           = options[:traits]  || []
       @class_name       = options[:class]
       @default_strategy = options[:default_strategy]
-      @defined_traits   = []
-      @attribute_list   = build_attribute_list
-      @callbacks = []
+      @definition       = Definition.new(@name)
     end
 
-    delegate :declare_attribute, :to => :@attribute_list
+    delegate :add_callback, :declare_attribute, :to_create, :define_trait,
+             :defined_traits, :trait_by_name, :to => :@definition
 
     def factory_name
       $stderr.puts "DEPRECATION WARNING: factory.factory_name is deprecated; use factory.name instead."
       name
     end
 
-    def add_callback(callback)
-      @callbacks << callback
-    end
-
     def build_class #:nodoc:
       @build_class ||= class_name.to_s.camelize.constantize
     end
@@ -38,19 +33,15 @@ def default_strategy #:nodoc:
     end
 
     def allow_overrides
-      @attribute_list.overridable
+      attribute_list.overridable
       self
     end
 
-    def define_trait(trait)
-      @defined_traits << trait
-    end
-
     def run(proxy_class, overrides, &block) #:nodoc:
       runner_options = {
         :attributes  => attributes,
         :callbacks   => callbacks,
-        :to_create   => @to_create_block,
+        :to_create   => to_create,
         :build_class => build_class,
         :proxy_class => proxy_class
       }
@@ -68,16 +59,6 @@ def associations
       attributes.select {|attribute| attribute.association? }
     end
 
-    def trait_by_name(name)
-      if existing_attribute = trait_for(name)
-        existing_attribute
-      elsif parent
-        parent.trait_by_name(name)
-      else
-        FactoryGirl.trait_by_name(name)
-      end
-    end
-
     # Names for this factory, including aliases.
     #
     # Example:
@@ -107,13 +88,12 @@ def names
       [name] + @aliases
     end
 
-    def to_create(&block)
-      @to_create_block = block
-    end
-
     def compile
-      parent.compile if parent
-      @attribute_list.ensure_compiled
+      if parent
+        parent.defined_traits.each {|trait| define_trait(trait) }
+        parent.compile
+      end
+      attribute_list.ensure_compiled
     end
 
     protected
@@ -124,18 +104,18 @@ def class_name #:nodoc:
 
     def attributes
       compile
-      build_attribute_list.tap do |list|
+      AttributeList.new(@name).tap do |list|
         traits.each do |trait|
           list.apply_attribute_list(trait.attributes)
         end
 
-        list.apply_attribute_list(@attribute_list)
+        list.apply_attribute_list(attribute_list)
         list.apply_attribute_list(parent.attributes) if parent
       end
     end
 
     def callbacks
-      [@callbacks].tap do |result|
+      [traits.map(&:callbacks), @definition.callbacks].tap do |result|
         result.unshift(*parent.callbacks) if parent
       end.flatten
     end
@@ -156,17 +136,13 @@ def traits
       @traits.reverse.map { |name| trait_by_name(name) }
     end
 
-    def trait_for(name)
-      @defined_traits.detect {|trait| trait.name == name }
-    end
-
     def parent
       return unless @parent
       FactoryGirl.factory_by_name(@parent)
     end
 
-    def build_attribute_list
-      AttributeList.new(@name)
+    def attribute_list
+      @definition.attribute_list
     end
 
     class Runner

lib/factory_girl/syntax/default.rb

@@ -18,7 +18,7 @@ def self.run(block)
 
         def factory(name, options = {}, &block)
           factory = Factory.new(name, options)
-          proxy = FactoryGirl::DefinitionProxy.new(factory)
+          proxy = FactoryGirl::DefinitionProxy.new(factory.definition)
           proxy.instance_eval(&block) if block_given?
 
           FactoryGirl.register_factory(factory)
@@ -44,9 +44,8 @@ def self.run(block)
 
         def factory(name, options = {}, &block)
           factory = FactoryGirl.factory_by_name(name).allow_overrides
-          proxy = FactoryGirl::DefinitionProxy.new(factory)
+          proxy = FactoryGirl::DefinitionProxy.new(factory.definition)
           proxy.instance_eval(&block)
-          factory.compile
         end
       end
     end

lib/factory_girl/trait.rb

@@ -4,18 +4,19 @@ class Trait
 
     def initialize(name, &block) #:nodoc:
       @name = name
-      @attribute_list = AttributeList.new
       @block = block
+      @definition = Definition.new
 
-      proxy = FactoryGirl::DefinitionProxy.new(self)
+      proxy = FactoryGirl::DefinitionProxy.new(@definition)
       proxy.instance_eval(&@block) if block_given?
     end
 
-    delegate :declare_attribute, :to => :@attribute_list
+    delegate :add_callback, :declare_attribute, :to_create, :define_trait,
+             :callbacks, :to => :@definition
 
     def attributes
-      @attribute_list.ensure_compiled
-      @attribute_list
+      attribute_list.ensure_compiled
+      attribute_list
     end
 
     def names
@@ -29,5 +30,11 @@ def ==(other)
 
     protected
     attr_reader :block
+
+    private
+
+    def attribute_list
+      @definition.attribute_list
+    end
   end
 end

spec/acceptance/traits_spec.rb

@@ -168,3 +168,35 @@
     it { expect { subject }.to raise_error(ArgumentError, "Trait not registered: admin_trait") }
   end
 end
+
+describe "traits with callbacks" do
+  before do
+    define_model("User", :name => :string)
+
+    FactoryGirl.define do
+      factory :user do
+        name "John"
+
+        trait :great do
+          after_create {|user| user.name.upcase! }
+        end
+
+        factory :caps_user, :traits => [:great]
+
+        factory :caps_user_implicit_trait do
+          great
+        end
+      end
+    end
+  end
+
+  context "when the factory has a trait passed via arguments" do
+    subject    { FactoryGirl.create(:caps_user) }
+    its(:name) { should == "JOHN" }
+  end
+
+  context "when the factory has an implicit trait" do
+    subject    { FactoryGirl.create(:caps_user_implicit_trait) }
+    its(:name) { pending }
+  end
+end