1 files changed, 84 insertions, 89 deletions

diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index 73761520f7..8f165fb1dc 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -1,4 +1,5 @@
 require 'active_support/core_ext/enumerable'
+require 'active_support/core_ext/string/filters'
 require 'mutex_m'
 require 'thread_safe'
 
@@ -18,6 +19,8 @@ module ActiveRecord
       include TimeZoneConversion
       include Dirty
       include Serialization
+
+      delegate :column_for_attribute, to: :class
     end
 
     AttrNames = Module.new {
@@ -29,6 +32,8 @@ module ActiveRecord
       end
     }
 
+    BLACKLISTED_CLASS_METHODS = %w(private public protected allocate new name parent superclass)
+
     class AttributeMethodCache
       def initialize
         @module = Module.new
@@ -46,9 +51,15 @@ module ActiveRecord
       end
 
       private
-      def method_body; raise NotImplementedError; end
+
+      # Override this method in the subclasses for method body.
+      def method_body(method_name, const_name)
+        raise NotImplementedError, "Subclasses must implement a method_body(method_name, const_name) method."
+      end
     end
 
+    class GeneratedAttributeMethods < Module; end # :nodoc:
+
     module ClassMethods
       def inherited(child_class) #:nodoc:
         child_class.initialize_generated_modules
@@ -56,15 +67,18 @@ module ActiveRecord
       end
 
       def initialize_generated_modules # :nodoc:
-        @generated_attribute_methods = Module.new { extend Mutex_m }
+        @generated_attribute_methods = GeneratedAttributeMethods.new { extend Mutex_m }
         @attribute_methods_generated = false
         include @generated_attribute_methods
+
+        super
       end
 
       # Generates all the attribute related methods for columns in the database
       # accessors, mutators and query methods.
       def define_attribute_methods # :nodoc:
-        # Use a mutex; we don't want two thread simultaneously trying to define
+        return false if @attribute_methods_generated
+        # Use a mutex; we don't want two threads simultaneously trying to define
         # attribute methods.
         generated_attribute_methods.synchronize do
           return false if @attribute_methods_generated
@@ -77,7 +91,7 @@ module ActiveRecord
 
       def undefine_attribute_methods # :nodoc:
         generated_attribute_methods.synchronize do
-          super if @attribute_methods_generated
+          super if defined?(@attribute_methods_generated) && @attribute_methods_generated
           @attribute_methods_generated = false
         end
       end
@@ -98,28 +112,30 @@ module ActiveRecord
       #   # => false
       def instance_method_already_implemented?(method_name)
         if dangerous_attribute_method?(method_name)
-          raise DangerousAttributeError, "#{method_name} is defined by Active Record"
+          raise DangerousAttributeError, "#{method_name} is defined by Active Record. Check to make sure that you don't have an attribute or method with the same name."
         end
 
         if superclass == Base
           super
         else
-          # If B < A and A defines its own attribute method, then we don't want to overwrite that.
-          defined = method_defined_within?(method_name, superclass, superclass.generated_attribute_methods)
-          defined && !ActiveRecord::Base.method_defined?(method_name) || super
+          # If ThisClass < ... < SomeSuperClass < ... < Base and SomeSuperClass
+          # defines its own attribute method, then we don't want to overwrite that.
+          defined = method_defined_within?(method_name, superclass, Base) &&
+            ! superclass.instance_method(method_name).owner.is_a?(GeneratedAttributeMethods)
+          defined || super
         end
       end
 
-      # A method name is 'dangerous' if it is already defined by Active Record, but
+      # A method name is 'dangerous' if it is already (re)defined by Active Record, but
       # not by any ancestors. (So 'puts' is not dangerous but 'save' is.)
       def dangerous_attribute_method?(name) # :nodoc:
         method_defined_within?(name, Base)
       end
 
-      def method_defined_within?(name, klass, sup = klass.superclass) # :nodoc:
+      def method_defined_within?(name, klass, superklass = klass.superclass) # :nodoc:
         if klass.method_defined?(name) || klass.private_method_defined?(name)
-          if sup.method_defined?(name) || sup.private_method_defined?(name)
-            klass.instance_method(name).owner != sup.instance_method(name).owner
+          if superklass.method_defined?(name) || superklass.private_method_defined?(name)
+            klass.instance_method(name).owner != superklass.instance_method(name).owner
           else
             true
           end
@@ -128,14 +144,22 @@ module ActiveRecord
         end
       end
 
-      def find_generated_attribute_method(method_name) # :nodoc:
-        klass = self
-        until klass == Base
-          gen_methods = klass.generated_attribute_methods
-          return gen_methods.instance_method(method_name) if method_defined_within?(method_name, gen_methods, Object)
-          klass = klass.superclass
+      # A class method is 'dangerous' if it is already (re)defined by Active Record, but
+      # not by any ancestors. (So 'puts' is not dangerous but 'new' is.)
+      def dangerous_class_method?(method_name)
+        BLACKLISTED_CLASS_METHODS.include?(method_name.to_s) || class_method_defined_within?(method_name, Base)
+      end
+
+      def class_method_defined_within?(name, klass, superklass = klass.superclass) # :nodoc:
+        if klass.respond_to?(name, true)
+          if superklass.respond_to?(name, true)
+            klass.method(name).owner != superklass.method(name).owner
+          else
+            true
+          end
+        else
+          false
         end
-        nil
       end
 
       # Returns +true+ if +attribute+ is an attribute method and table exists,
@@ -166,29 +190,31 @@ module ActiveRecord
             []
           end
       end
-    end
 
-    # If we haven't generated any methods yet, generate them, then
-    # see if we've created the method we're looking for.
-    def method_missing(method, *args, &block) # :nodoc:
-      self.class.define_attribute_methods
-      if respond_to_without_attributes?(method)
-        # make sure to invoke the correct attribute method, as we might have gotten here via a `super`
-        # call in a overwritten attribute method
-        if attribute_method = self.class.find_generated_attribute_method(method)
-          # this is probably horribly slow, but should only happen at most once for a given AR class
-          attribute_method.bind(self).call(*args, &block)
-        else
-          send(method, *args, &block)
+      # Returns the column object for the named attribute.
+      # Returns a +ActiveRecord::ConnectionAdapters::NullColumn+ if the
+      # named attribute does not exist.
+      #
+      #   class Person < ActiveRecord::Base
+      #   end
+      #
+      #   person = Person.new
+      #   person.column_for_attribute(:name) # the result depends on the ConnectionAdapter
+      #   # => #<ActiveRecord::ConnectionAdapters::Column:0x007ff4ab083980 @name="name", @sql_type="varchar(255)", @null=true, ...>
+      #
+      #   person.column_for_attribute(:nothing)
+      #   # => #<ActiveRecord::ConnectionAdapters::NullColumn:0xXXX @name=nil, @sql_type=nil, @cast_type=#<Type::Value>, ...>
+      def column_for_attribute(name)
+        name = name.to_s
+        columns_hash.fetch(name) do
+          ConnectionAdapters::NullColumn.new(name)
         end
-      else
-        super
       end
     end
 
     # A Person object with a name attribute can ask <tt>person.respond_to?(:name)</tt>,
     # <tt>person.respond_to?(:name=)</tt>, and <tt>person.respond_to?(:name?)</tt>
-    # which will all return +true+. It also define the attribute methods if they have
+    # which will all return +true+. It also defines the attribute methods if they have
     # not been generated.
     #
     #   class Person < ActiveRecord::Base
@@ -203,18 +229,14 @@ module ActiveRecord
     #   person.respond_to('age?')   # => true
     #   person.respond_to(:nothing) # => false
     def respond_to?(name, include_private = false)
+      return false unless super
       name = name.to_s
-      self.class.define_attribute_methods
-      result = super
-
-      # If the result is false the answer is false.
-      return false unless result
 
       # If the result is true then check for the select case.
       # For queries selecting a subset of columns, return false for unselected columns.
       # We check defined?(@attributes) not to issue warnings if called on objects that
       # have been allocated but not yet initialized.
-      if defined?(@attributes) && @attributes.any? && self.class.column_names.include?(name)
+      if defined?(@attributes) && self.class.column_names.include?(name)
         return has_attribute?(name)
       end
 
@@ -231,7 +253,7 @@ module ActiveRecord
     #   person.has_attribute?('age')    # => true
     #   person.has_attribute?(:nothing) # => false
     def has_attribute?(attr_name)
-      @attributes.has_key?(attr_name.to_s)
+      @attributes.key?(attr_name.to_s)
     end
 
     # Returns an array of names for the attributes available on this object.
@@ -255,15 +277,13 @@ module ActiveRecord
     #   person.attributes
     #   # => {"id"=>3, "created_at"=>Sun, 21 Oct 2012 04:53:04, "updated_at"=>Sun, 21 Oct 2012 04:53:04, "name"=>"Francesco", "age"=>22}
     def attributes
-      attribute_names.each_with_object({}) { |name, attrs|
-        attrs[name] = read_attribute(name)
-      }
+      @attributes.to_hash
     end
 
     # Returns an <tt>#inspect</tt>-like string for the value of the
-    # attribute +attr_name+. String attributes are truncated upto 50
+    # attribute +attr_name+. String attributes are truncated up to 50
     # characters, Date and Time attributes are returned in the
-    # <tt>:db</tt> format, Array attributes are truncated upto 10 values.
+    # <tt>:db</tt> format, Array attributes are truncated up to 10 values.
     # Other attributes return the value of <tt>#inspect</tt> without
     # modification.
     #
@@ -300,39 +320,24 @@ module ActiveRecord
     #   class Task < ActiveRecord::Base
     #   end
     #
-    #   person = Task.new(title: '', is_done: false)
-    #   person.attribute_present?(:title)   # => false
-    #   person.attribute_present?(:is_done) # => true
-    #   person.name = 'Francesco'
-    #   person.is_done = true
-    #   person.attribute_present?(:title)   # => true
-    #   person.attribute_present?(:is_done) # => true
+    #   task = Task.new(title: '', is_done: false)
+    #   task.attribute_present?(:title)   # => false
+    #   task.attribute_present?(:is_done) # => true
+    #   task.title = 'Buy milk'
+    #   task.is_done = true
+    #   task.attribute_present?(:title)   # => true
+    #   task.attribute_present?(:is_done) # => true
     def attribute_present?(attribute)
-      value = read_attribute(attribute)
+      value = _read_attribute(attribute)
       !value.nil? && !(value.respond_to?(:empty?) && value.empty?)
     end
 
-    # Returns the column object for the named attribute. Returns +nil+ if the
-    # named attribute not exists.
-    #
-    #   class Person < ActiveRecord::Base
-    #   end
-    #
-    #   person = Person.new
-    #   person.column_for_attribute(:name) # the result depends on the ConnectionAdapter
-    #   # => #<ActiveRecord::ConnectionAdapters::SQLite3Column:0x007ff4ab083980 @name="name", @sql_type="varchar(255)", @null=true, ...>
-    #
-    #   person.column_for_attribute(:nothing)
-    #   # => nil
-    def column_for_attribute(name)
-      # FIXME: should this return a null object for columns that don't exist?
-      self.class.columns_hash[name.to_s]
-    end
-
     # Returns the value of the attribute identified by <tt>attr_name</tt> after it has been typecast (for example,
     # "2004-12-12" in a date column is cast to a date object, like Date.new(2004, 12, 12)). It raises
     # <tt>ActiveModel::MissingAttributeError</tt> if the identified attribute is missing.
     #
+    # Note: +:id+ is always present.
+    #
     # Alias for the <tt>read_attribute</tt> method.
     #
     #   class Person < ActiveRecord::Base
@@ -366,13 +371,6 @@ module ActiveRecord
 
     protected
 
-    def clone_attributes(reader_method = :read_attribute, attributes = {}) # :nodoc:
-      attribute_names.each do |name|
-        attributes[name] = clone_attribute_value(reader_method, name)
-      end
-      attributes
-    end
-
     def clone_attribute_value(reader_method, attribute_name) # :nodoc:
       value = send(reader_method, attribute_name)
       value.duplicable? ? value.clone : value
@@ -390,7 +388,7 @@ module ActiveRecord
 
     def attribute_method?(attr_name) # :nodoc:
       # We check defined? because Syck calls respond_to? before actually calling initialize.
-      defined?(@attributes) && @attributes.include?(attr_name)
+      defined?(@attributes) && @attributes.key?(attr_name)
     end
 
     private
@@ -409,16 +407,16 @@ module ActiveRecord
 
     # Filters the primary keys and readonly attributes from the attribute names.
     def attributes_for_update(attribute_names)
-      attribute_names.select do |name|
-        column_for_attribute(name) && !readonly_attribute?(name)
+      attribute_names.reject do |name|
+        readonly_attribute?(name)
       end
     end
 
     # Filters out the primary keys, from the attribute names, when the primary
     # key is to be generated (e.g. the id attribute has no value).
     def attributes_for_create(attribute_names)
-      attribute_names.select do |name|
-        column_for_attribute(name) && !(pk_attribute?(name) && id.nil?)
+      attribute_names.reject do |name|
+        pk_attribute?(name) && id.nil?
       end
     end
 
@@ -427,14 +425,11 @@ module ActiveRecord
     end
 
     def pk_attribute?(name)
-      column_for_attribute(name).primary
+      name == self.class.primary_key
     end
 
     def typecasted_attribute_value(name)
-      # FIXME: we need @attributes to be used consistently.
-      # If the values stored in @attributes were already typecasted, this code
-      # could be simplified
-      read_attribute(name)
+      _read_attribute(name)
     end
   end
 end