Restore DangerousAttributeError

3 files changed, 27 insertions, 3 deletions

diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index 8370c212ca..c7e6fe6a29 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -85,10 +85,14 @@ module ActiveRecord
       end
 
       # Checks whether the method is defined in the model or any of its subclasses
-      # that also derive from Active Record.
+      # that also derive from Active Record. Raises DangerousAttributeError if the
+      # method is defined by Active Record though.
       def instance_method_already_implemented?(method_name)
         method_name = method_name.to_s
-        @_defined_class_methods ||= ancestors.first(ancestors.index(ActiveRecord::Base)).sum([]) { |m| m.public_instance_methods(false) | m.private_instance_methods(false) | m.protected_instance_methods(false) }.map(&:to_s).to_set
+        return true if method_name =~ /^id(=$|\?$|_before_type_cast$|$)/
+        @_defined_class_methods         ||= ancestors.first(ancestors.index(ActiveRecord::Base)).sum([]) { |m| m.public_instance_methods(false) | m.private_instance_methods(false) | m.protected_instance_methods(false) }.map {|m| m.to_s }.to_set
+        @@_defined_activerecord_methods ||= (ActiveRecord::Base.public_instance_methods(false) | ActiveRecord::Base.private_instance_methods(false) | ActiveRecord::Base.protected_instance_methods(false)).map{|m| m.to_s }.to_set
+        raise DangerousAttributeError, "#{method_name} is defined by ActiveRecord" if @@_defined_activerecord_methods.include?(method_name)
         @_defined_class_methods.include?(method_name)
       end
 
@@ -105,7 +109,9 @@ module ActiveRecord
 
         # Evaluate the definition for an attribute related method
         def evaluate_attribute_method(attr_name, method_definition, method_name)
-          generated_methods << method_name
+          unless method_name.to_s == primary_key.to_s
+            generated_methods << method_name
+          end
 
           begin
             class_eval(method_definition, __FILE__, __LINE__)
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index b93dfaf987..7de5bf8a77 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -144,6 +144,10 @@ module ActiveRecord #:nodoc:
   class Rollback < ActiveRecordError
   end
 
+  # Raised when attribute has a name reserved by Active Record (when attribute has name of one of Active Record instance methods).
+  class DangerousAttributeError < ActiveRecordError
+  end
+
   # Raised when you've tried to access a column which wasn't loaded by your finder.
   # Typically this is because <tt>:select</tt> has been specified.
   class MissingAttributeError < NoMethodError
diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb
index 77e406bbba..183be1e2f9 100644
--- a/activerecord/test/cases/attribute_methods_test.rb
+++ b/activerecord/test/cases/attribute_methods_test.rb
@@ -74,6 +74,10 @@ class AttributeMethodsTest < ActiveRecord::TestCase
     end
   end
 
+  def test_primary_key_implemented
+    assert Class.new(ActiveRecord::Base).instance_method_already_implemented?('id')
+  end
+
   def test_defined_kernel_methods_implemented_in_model
     %w(test name display y).each do |method|
       klass = Class.new ActiveRecord::Base
@@ -92,6 +96,16 @@ class AttributeMethodsTest < ActiveRecord::TestCase
     end
   end
 
+  def test_raises_dangerous_attribute_error_when_defining_activerecord_method_in_model
+    %w(save create_or_update).each do |method|
+      klass = Class.new ActiveRecord::Base
+      klass.class_eval "def #{method}() 'defined #{method}' end"
+      assert_raise ActiveRecord::DangerousAttributeError do
+        klass.instance_method_already_implemented?(method)
+      end
+    end
+  end
+
   def test_only_time_related_columns_are_meant_to_be_cached_by_default
     expected = %w(datetime timestamp time date).sort
     assert_equal expected, ActiveRecord::Base.attribute_types_cached_by_default.map(&:to_s).sort