Wrap up DeclarationList

DeclarationList knows how to generate an attribute
list, which never really made sense outside of being generated from
declarations. Now, the declaration list builds a list of attributes
which is combined in Factory#attributes with attributes from traits and
its parents.

lib/factory_girl/attribute_list.rb

@@ -2,19 +2,9 @@ module FactoryGirl
   class AttributeList
     include Enumerable
 
-    attr_reader :declarations
-
     def initialize(name = nil)
-      @name         = name
-      @attributes   = {}
-      @declarations = DeclarationList.new
-      @overridable  = false
-      @compiled     = false
-    end
-
-    def declare_attribute(declaration)
-      @declarations << declaration
-      declaration
+      @name       = name
+      @attributes = {}
     end
 
     def define_attribute(attribute)
@@ -28,44 +18,21 @@ def each(&block)
       flattened_attributes.each(&block)
     end
 
-    def ensure_compiled
-      compile unless @compiled
-    end
-
-    def apply_attribute_list(attributes_to_apply)
+    def apply_attributes(attributes_to_apply)
       new_attributes = []
 
       attributes_to_apply.each do |attribute|
-        new_attribute = if !overridable? && defined_attribute = find_attribute(attribute.name)
-          defined_attribute
-        else
-          attribute
-        end
-
+        new_attribute = find_attribute(attribute.name) || attribute
         delete_attribute(attribute.name)
         new_attributes << new_attribute
       end
 
       prepend_attributes new_attributes
     end
 
-    def overridable
-      @compiled = false
-      @overridable = true
-    end
-
     private
 
-    def compile
-      @declarations.to_attributes.each do |attribute|
-        define_attribute(attribute)
-      end
-      @compiled = true
-    end
-
     def add_attribute(attribute)
-      delete_attribute(attribute.name) if overridable?
-
       @attributes[attribute.priority] ||= []
       @attributes[attribute.priority] << attribute
       attribute
@@ -86,7 +53,7 @@ def flattened_attributes
     end
 
     def ensure_attribute_not_defined!(attribute)
-      if !overridable? && attribute_defined?(attribute.name)
+      if attribute_defined?(attribute.name)
         raise AttributeDefinitionError, "Attribute already defined: #{attribute.name}"
       end
     end
@@ -114,9 +81,5 @@ def delete_attribute(attribute_name)
         end
       end
     end
-
-    def overridable?
-      @overridable
-    end
   end
 end

lib/factory_girl/declaration_list.rb

@@ -1,15 +1,48 @@
 module FactoryGirl
   class DeclarationList
-    def initialize
+    include Enumerable
+
+    def initialize(name = nil)
       @declarations = []
+      @name         = name
+      @overridable  = false
+    end
+
+    def declare_attribute(declaration)
+      delete_declaration(declaration) if overridable?
+
+      @declarations << declaration
+      declaration
+    end
+
+    def overridable
+      @overridable = true
+    end
+
+    def attribute_list
+      AttributeList.new(@name).tap do |list|
+        to_attributes.each do |attribute|
+          list.define_attribute(attribute)
+        end
+      end
+    end
+
+    def each(&block)
+      @declarations.each(&block)
+    end
+
+    private
+
+    def delete_declaration(declaration)
+      @declarations.delete_if {|decl| decl.name == declaration.name }
     end
 
     def to_attributes
       @declarations.inject([]) {|result, declaration| result += declaration.to_attributes }
     end
 
-    def method_missing(name, *args, &block)
-      @declarations.send(name, *args, &block)
+    def overridable?
+      @overridable
     end
   end
 end

lib/factory_girl/definition.rb

@@ -1,16 +1,29 @@
 module FactoryGirl
   class Definition
-    attr_reader :callbacks, :defined_traits, :attribute_list
+    attr_reader :callbacks, :defined_traits, :declarations
 
     def initialize(name = nil)
-      @attribute_list = AttributeList.new(name)
+      @declarations   = DeclarationList.new(name)
       @callbacks      = []
       @defined_traits = []
       @to_create      = nil
       @traits         = []
     end
 
-    delegate :declare_attribute, :to => :attribute_list
+    delegate :declare_attribute, :to => :declarations
+
+    def attributes
+      @attributes ||= declarations.attribute_list
+    end
+
+    def compile
+      attributes
+    end
+
+    def overridable
+      declarations.overridable
+      self
+    end
 
     def traits
       @traits.reverse.map { |name| trait_by_name(name) }

lib/factory_girl/factory.rb

@@ -33,11 +33,6 @@ def default_strategy #:nodoc:
       @default_strategy || parent.default_strategy || :create
     end
 
-    def allow_overrides
-      attribute_list.overridable
-      self
-    end
-
     def run(proxy_class, overrides, &block) #:nodoc:
       runner_options = {
         :attributes  => attributes,
@@ -92,7 +87,7 @@ def names
     def compile
       parent.defined_traits.each {|trait| define_trait(trait) }
       parent.compile
-      attribute_list.ensure_compiled
+      @definition.compile
     end
 
     protected
@@ -105,11 +100,11 @@ def attributes
       compile
       AttributeList.new(@name).tap do |list|
         traits.each do |trait|
-          list.apply_attribute_list(trait.attributes)
+          list.apply_attributes(trait.attributes)
         end
 
-        list.apply_attribute_list(attribute_list)
-        list.apply_attribute_list(parent.attributes)
+        list.apply_attributes(@definition.attributes)
+        list.apply_attributes(parent.attributes)
       end
     end
 
@@ -137,10 +132,6 @@ def parent
       end
     end
 
-    def attribute_list
-      @definition.attribute_list
-    end
-
     class Runner
       def initialize(options = {})
         @attributes  = options[:attributes]

lib/factory_girl/null_factory.rb

@@ -6,14 +6,10 @@ def initialize
       @definition = Definition.new
     end
 
-    delegate :defined_traits, :callbacks, :to => :definition
+    delegate :defined_traits, :callbacks, :attributes, :to => :definition
 
     def compile; end
     def default_strategy; end
     def class_name; end
-
-    def attributes
-      AttributeList.new
-    end
   end
 end

lib/factory_girl/syntax/default.rb

@@ -43,8 +43,8 @@ def self.run(block)
         end
 
         def factory(name, options = {}, &block)
-          factory = FactoryGirl.factory_by_name(name).allow_overrides
-          proxy = FactoryGirl::DefinitionProxy.new(factory.definition)
+          factory = FactoryGirl.factory_by_name(name)
+          proxy = FactoryGirl::DefinitionProxy.new(factory.definition.overridable)
           proxy.instance_eval(&block)
         end
       end

lib/factory_girl/trait.rb

@@ -12,12 +12,7 @@ def initialize(name, &block) #:nodoc:
     end
 
     delegate :add_callback, :declare_attribute, :to_create, :define_trait,
-             :callbacks, :to => :@definition
-
-    def attributes
-      attribute_list.ensure_compiled
-      attribute_list
-    end
+             :callbacks, :attributes, :to => :@definition
 
     def names
       [@name]
@@ -30,11 +25,5 @@ def ==(other)
 
     protected
     attr_reader :block
-
-    private
-
-    def attribute_list
-      @definition.attribute_list
-    end
   end
 end

spec/factory_girl/attribute_list_spec.rb

@@ -1,14 +1,5 @@
 require "spec_helper"
 
-describe FactoryGirl::AttributeList, "overridable" do
-  it { should_not be_overridable }
-
-  it "can set itself as overridable" do
-    subject.overridable
-    subject.should be_overridable
-  end
-end
-
 describe FactoryGirl::AttributeList, "#define_attribute" do
   let(:static_attribute)  { FactoryGirl::Attribute::Static.new(:full_name, "value", false) }
   let(:dynamic_attribute) { FactoryGirl::Attribute::Dynamic.new(:email, false, lambda {|u| "#{u.full_name}@example.com" }) }
@@ -31,18 +22,6 @@
       2.times { subject.define_attribute(static_attribute) }
     }.to raise_error(FactoryGirl::AttributeDefinitionError, "Attribute already defined: full_name")
   end
-
-  context "when set as overridable" do
-    let(:static_attribute_with_same_name) { FactoryGirl::Attribute::Static.new(:full_name, "overridden value", false) }
-    before { subject.overridable }
-
-    it "redefines the attribute if the name already exists" do
-      subject.define_attribute(static_attribute)
-      subject.define_attribute(static_attribute_with_same_name)
-
-      subject.to_a.should == [static_attribute_with_same_name]
-    end
-  end
 end
 
 describe FactoryGirl::AttributeList, "#define_attribute with a named attribute list" do
@@ -60,7 +39,7 @@
   end
 end
 
-describe FactoryGirl::AttributeList, "#apply_attribute_list" do
+describe FactoryGirl::AttributeList, "#apply_attributes" do
   let(:full_name_attribute) { FactoryGirl::Attribute::Static.new(:full_name, "John Adams", false) }
   let(:city_attribute)      { FactoryGirl::Attribute::Static.new(:city, "Boston", false) }
   let(:email_attribute)     { FactoryGirl::Attribute::Dynamic.new(:email, false, lambda {|model| "#{model.full_name}@example.com" }) }
@@ -74,59 +53,28 @@ def list(*attributes)
 
   it "prepends applied attributes" do
     subject.define_attribute(full_name_attribute)
-    subject.apply_attribute_list(list(city_attribute))
+    subject.apply_attributes(list(city_attribute))
     subject.to_a.should == [city_attribute, full_name_attribute]
   end
 
   it "moves non-static attributes to the end of the list" do
     subject.define_attribute(full_name_attribute)
-    subject.apply_attribute_list(list(city_attribute, email_attribute))
+    subject.apply_attributes(list(city_attribute, email_attribute))
     subject.to_a.should == [city_attribute, full_name_attribute, email_attribute]
   end
 
   it "maintains order of non-static attributes" do
     subject.define_attribute(full_name_attribute)
     subject.define_attribute(login_attribute)
-    subject.apply_attribute_list(list(city_attribute, email_attribute))
+    subject.apply_attributes(list(city_attribute, email_attribute))
     subject.to_a.should == [city_attribute, full_name_attribute, email_attribute, login_attribute]
   end
 
   it "doesn't overwrite attributes that are already defined" do
     subject.define_attribute(full_name_attribute)
     attribute_with_same_name = FactoryGirl::Attribute::Static.new(:full_name, "Benjamin Franklin", false)
 
-    subject.apply_attribute_list(list(attribute_with_same_name))
+    subject.apply_attributes(list(attribute_with_same_name))
     subject.to_a.should == [full_name_attribute]
   end
-
-  context "when set as overridable" do
-    before { subject.overridable }
-
-    it "prepends applied attributes" do
-      subject.define_attribute(full_name_attribute)
-      subject.apply_attribute_list(list(city_attribute))
-      subject.to_a.should == [city_attribute, full_name_attribute]
-    end
-
-    it "moves non-static attributes to the end of the list" do
-      subject.define_attribute(full_name_attribute)
-      subject.apply_attribute_list(list(city_attribute, email_attribute))
-      subject.to_a.should == [city_attribute, full_name_attribute, email_attribute]
-    end
-
-    it "maintains order of non-static attributes" do
-      subject.define_attribute(full_name_attribute)
-      subject.define_attribute(login_attribute)
-      subject.apply_attribute_list(list(city_attribute, email_attribute))
-      subject.to_a.should == [city_attribute, full_name_attribute, email_attribute, login_attribute]
-    end
-
-    it "overwrites attributes that are already defined" do
-      subject.define_attribute(full_name_attribute)
-      attribute_with_same_name = FactoryGirl::Attribute::Static.new(:full_name, "Benjamin Franklin", false)
-
-      subject.apply_attribute_list(list(attribute_with_same_name))
-      subject.to_a.should == [attribute_with_same_name]
-    end
-  end
 end