Merge commit 'rails/master'

actionpack/lib/action_view/base.rb

@@ -236,7 +236,12 @@ module ActionView #:nodoc:
       # they are in AC.
       if controller.class.respond_to?(:_helper_serial)
         klass = @views[controller.class._helper_serial] ||= Class.new(self) do
-          Subclasses.const_set(controller.class.name.gsub(/::/, '__'), self)
+          name = controller.class.name.gsub(/::/, '__')
+
+          Subclasses.class_eval do
+            remove_const(name) if const_defined?(name)
+            const_set(name, self)
+          end
 
           if controller.respond_to?(:_helpers)
             include controller._helpers

activemodel/lib/active_model/attribute_methods.rb

@@ -1,3 +1,6 @@
+require 'active_support/core_ext/hash/keys'
+require 'active_support/core_ext/class/inheritable_attributes'
+
 module ActiveModel
   class MissingAttributeError < NoMethodError
   end
@@ -162,6 +165,7 @@ module ActiveModel
             end
           end
         end
+        @attribute_methods_generated = true
       end
 
       def undefine_attribute_methods
@@ -173,7 +177,6 @@ module ActiveModel
 
       def generated_attribute_methods #:nodoc:
         @generated_attribute_methods ||= begin
-          @attribute_methods_generated = true
           mod = Module.new
           include mod
           mod
@@ -219,7 +222,7 @@ module ActiveModel
         end
 
         def attribute_method_matchers #:nodoc:
-          @@attribute_method_matchers ||= []
+          read_inheritable_attribute(:attribute_method_matchers) || write_inheritable_attribute(:attribute_method_matchers, [])
         end
     end
 

activemodel/lib/active_model/lint.rb

@@ -1,6 +1,6 @@
 # You can test whether an object is compliant with the ActiveModel API by
-# calling ActiveModel::Lint.test(object). It will emit a Test::Unit
-# output that tells you whether your object is fully compliant, or if not,
+# including ActiveModel::Lint::Tests in your TestCase. It will included
+# tests that tell you whether your object is fully compliant, or if not,
 # which aspects of the API are not implemented.
 #
 # These tests do not attempt to determine the semantic correctness of the
@@ -12,36 +12,15 @@
 # call to to_model. It is perfectly fine for to_model to return self.
 module ActiveModel
   module Lint
-    def self.test(object, verbosity = 2, output = STDOUT)
-      require "test/unit"
-      require "test/unit/ui/console/testrunner"
-
-      test_class = Class.new(::Test::Unit::TestCase) do
-        include Test
-
-        define_method(:setup) do
-          assert object.respond_to?(:to_model), "The object should respond_to :to_model"
-          @object = object.to_model
-          super
-        end
-      end
-
-      ::Test::Unit::UI::Console::TestRunner.new(test_class, verbosity, output).start
-    end
-
-    module Test
-      def assert_boolean(name, result)
-        assert result == true || result == false, "#{name} should be a boolean"
-      end
-
+    module Tests
       # valid?
       # ------
       #
       # Returns a boolean that specifies whether the object is in a valid or invalid
       # state.
       def test_valid?
-        assert @object.respond_to?(:valid?), "The model should respond to valid?"
-        assert_boolean "valid?", @object.valid?
+        assert model.respond_to?(:valid?), "The model should respond to valid?"
+        assert_boolean model.valid?, "valid?"
       end
 
       # new_record?
@@ -53,13 +32,13 @@ module ActiveModel
       # collection. If it is persisted, a form for the object will put PUTed to the
       # URL for the object.
       def test_new_record?
-        assert @object.respond_to?(:new_record?), "The model should respond to new_record?"
-        assert_boolean "new_record?", @object.new_record?
+        assert model.respond_to?(:new_record?), "The model should respond to new_record?"
+        assert_boolean model.new_record?, "new_record?"
       end
 
       def test_destroyed?
-        assert @object.respond_to?(:destroyed?), "The model should respond to destroyed?"
-        assert_boolean "destroyed?", @object.destroyed?
+        assert model.respond_to?(:destroyed?), "The model should respond to destroyed?"
+        assert_boolean model.destroyed?, "destroyed?"
       end
 
       # errors
@@ -67,29 +46,32 @@ module ActiveModel
       #
       # Returns an object that has :[] and :full_messages defined on it. See below
       # for more details.
-      def setup
-        assert @object.respond_to?(:errors), "The model should respond to errors"
-        @errors = @object.errors
+
+      # Returns an Array of Strings that are the errors for the attribute in
+      # question. If localization is used, the Strings should be localized
+      # for the current locale. If no error is present, this method should
+      # return an empty Array.
+      def test_errors_aref
+        assert model.respond_to?(:errors), "The model should respond to errors"
+        assert model.errors[:hello].is_a?(Array), "errors#[] should return an Array"
       end
 
-      # This module tests the #errors object
-      module Errors
-        # Returns an Array of Strings that are the errors for the attribute in
-        # question. If localization is used, the Strings should be localized
-        # for the current locale. If no error is present, this method should
-        # return an empty Array.
-        def test_errors_aref
-          assert @errors[:hello].is_a?(Array), "errors#[] should return an Array"
-        end
-
-        # Returns an Array of all error messages for the object. Each message
-        # should contain information about the field, if applicable.
-        def test_errors_full_messages
-          assert @errors.full_messages.is_a?(Array), "errors#full_messages should return an Array"
-        end
+      # Returns an Array of all error messages for the object. Each message
+      # should contain information about the field, if applicable.
+      def test_errors_full_messages
+        assert model.respond_to?(:errors), "The model should respond to errors"
+        assert model.errors.full_messages.is_a?(Array), "errors#full_messages should return an Array"
       end
 
-      include Errors
+      private
+        def model
+          assert @model.respond_to?(:to_model), "The object should respond_to to_model"
+          @model.to_model
+        end
+
+        def assert_boolean(result, name)
+          assert result == true || result == false, "#{name} should be a boolean"
+        end
     end
   end
 end

activemodel/test/cases/attribute_methods_test.rb

@@ -0,0 +1,46 @@
+require 'cases/helper'
+
+class ModelWithAttributes
+  include ActiveModel::AttributeMethods
+  
+  attribute_method_suffix ''
+
+  def attributes
+    { :foo => 'value of foo' }
+  end
+
+private
+  def attribute(name)
+    attributes[name.to_sym]
+  end
+end
+
+class ModelWithAttributes2
+  include ActiveModel::AttributeMethods
+  
+  attribute_method_suffix '_test'
+end
+
+class AttributeMethodsTest < ActiveModel::TestCase
+  test 'unrelated classes should not share attribute method matchers' do
+    assert_not_equal ModelWithAttributes.send(:attribute_method_matchers),
+                     ModelWithAttributes2.send(:attribute_method_matchers)
+  end
+
+  test '#define_attribute_methods generates attribute methods' do
+    ModelWithAttributes.define_attribute_methods([:foo])
+
+    assert ModelWithAttributes.attribute_methods_generated?
+    assert ModelWithAttributes.new.respond_to?(:foo)
+    assert_equal "value of foo", ModelWithAttributes.new.foo
+  end
+
+  test '#undefine_attribute_methods removes attribute methods' do
+    ModelWithAttributes.define_attribute_methods([:foo])
+    ModelWithAttributes.undefine_attribute_methods
+
+    assert !ModelWithAttributes.attribute_methods_generated?
+    assert !ModelWithAttributes.new.respond_to?(:foo)
+    assert_raises(NoMethodError) { ModelWithAttributes.new.foo }
+  end
+end

activemodel/test/cases/lint_test.rb

@@ -1,7 +1,9 @@
 require "cases/helper"
 
-class TestLint < ActiveModel::TestCase
-  class CompliantObject
+class LintTest < ActiveModel::TestCase
+  include ActiveModel::Lint::Tests
+
+  class CompliantModel
     def to_model
       self
     end
@@ -18,33 +20,7 @@ class TestLint < ActiveModel::TestCase
     end
   end
 
-  def assert_output(object, failures, errors, *test_names)
-    ActiveModel::Lint.test(object, 3, output = StringIO.new)
-    regex =  %r{#{failures} failures, #{errors} errors}
-    assert_match regex, output.string
-    
-    test_names.each do |test_name|
-      assert_match test_name, output.string
-    end
-  end
-  
-  def test_valid
-    assert_output(CompliantObject.new, 0, 0, /test_valid/)
-  end
-  
-  def test_new_record
-    assert_output(CompliantObject.new, 0, 0, /test_new_record?/)
-  end
-  
-  def test_destroyed
-    assert_output(CompliantObject.new, 0, 0, /test_destroyed/)
-  end
-  
-  def test_errors_aref
-    assert_output(CompliantObject.new, 0, 0, /test_errors_aref/)
-  end
-
-  def test_errors_full_messages
-    assert_output(CompliantObject.new, 0, 0, /test_errors_aref/)
+  def setup
+    @model = CompliantModel.new
   end
 end

activerecord/lib/active_record/base.rb

@@ -2456,6 +2456,29 @@ module ActiveRecord #:nodoc:
         result
       end
 
+      # Cloned objects have no id assigned and are treated as new records. Note that this is a "shallow" clone
+      # as it copies the object's attributes only, not its associations. The extent of a "deep" clone is
+      # application specific and is therefore left to the application to implement according to its need.
+      def initialize_copy(other)
+        # Think the assertion which fails if the after_initialize callback goes at the end of the method is wrong. The
+        # deleted clone method called new which therefore called the after_initialize callback. It then went on to copy
+        # over the attributes. But if it's copying the attributes afterwards then it hasn't finished initializing right?
+        # For example in the test suite the topic model's after_initialize method sets the author_email_address to
+        # test@test.com. I would have thought this would mean that all cloned models would have an author email address
+        # of test@test.com. However the test_clone test method seems to test that this is not the case. As a result the
+        # after_initialize callback has to be run *before* the copying of the atrributes rather than afterwards in order
+        # for all tests to pass. This makes no sense to me.
+        callback(:after_initialize) if respond_to_without_attributes?(:after_initialize)
+        cloned_attributes = other.clone_attributes(:read_attribute_before_type_cast)
+        cloned_attributes.delete(self.class.primary_key)
+        @attributes = cloned_attributes
+        clear_aggregation_cache
+        @attributes_cache = {}
+        @new_record = true
+        ensure_proper_type
+        self.class.send(:scope, :create).each { |att, value| self.send("#{att}=", value) } if self.class.send(:scoped?, :create)
+      end
+
       # Returns a String, which Action Pack uses for constructing an URL to this
       # object. The default implementation returns this record's id as a String,
       # or nil if this record's unsaved.
@@ -2580,19 +2603,6 @@ module ActiveRecord #:nodoc:
         freeze
       end
 
-      # Returns a clone of the record that hasn't been assigned an id yet and
-      # is treated as a new record.  Note that this is a "shallow" clone:
-      # it copies the object's attributes only, not its associations.
-      # The extent of a "deep" clone is application-specific and is therefore
-      # left to the application to implement according to its need.
-      def clone
-        attrs = clone_attributes(:read_attribute_before_type_cast)
-        attrs.delete(self.class.primary_key)
-        record = self.class.new
-        record.send :instance_variable_set, '@attributes', attrs
-        record
-      end
-
       # Returns an instance of the specified +klass+ with the attributes of the current record. This is mostly useful in relation to
       # single-table inheritance structures where you want a subclass to appear as the superclass. This can be used along with record
       # identification in Action Pack to allow, say, <tt>Client < Company</tt> to do something like render <tt>:partial => @client.becomes(Company)</tt>
@@ -2856,6 +2866,21 @@ module ActiveRecord #:nodoc:
         "#<#{self.class} #{attributes_as_nice_string}>"
       end
 
+    protected
+      def clone_attributes(reader_method = :read_attribute, attributes = {})
+        self.attribute_names.inject(attributes) do |attrs, name|
+          attrs[name] = clone_attribute_value(reader_method, name)
+          attrs
+        end
+      end
+
+      def clone_attribute_value(reader_method, attribute_name)
+        value = send(reader_method, attribute_name)
+        value.duplicable? ? value.clone : value
+      rescue TypeError, NoMethodError
+        value
+      end
+
     private
       def create_or_update
         raise ReadOnlyRecord if readonly?
@@ -3123,20 +3148,6 @@ module ActiveRecord #:nodoc:
         return string unless string.is_a?(String) && string =~ /^---/
         YAML::load(string) rescue string
       end
-
-      def clone_attributes(reader_method = :read_attribute, attributes = {})
-        self.attribute_names.inject(attributes) do |attrs, name|
-          attrs[name] = clone_attribute_value(reader_method, name)
-          attrs
-        end
-      end
-
-      def clone_attribute_value(reader_method, attribute_name)
-        value = send(reader_method, attribute_name)
-        value.duplicable? ? value.clone : value
-      rescue TypeError, NoMethodError
-        value
-      end
   end
 
   Base.class_eval do

activerecord/test/cases/base_test.rb

@@ -1352,7 +1352,7 @@ class BasicsTest < ActiveRecord::TestCase
     cloned_topic.title["a"] = "c"
     assert_equal "b", topic.title["a"]
 
-    #test if attributes set as part of after_initialize are cloned correctly
+    # test if attributes set as part of after_initialize are cloned correctly
     assert_equal topic.author_email_address, cloned_topic.author_email_address
 
     # test if saved clone object differs from original