Ticket #3005: association-duplicate-names-prevent-v2.patch

activerecord/test/associations_test.rb

@@ -258 +258 @@
-    assert_equal a, firm.account
-    assert_equal a, firm.account(true)
-  end
+
+  def test_prevent_naming_association_with_existing_name
+    firm = Firm.new
+    assert_raise ActiveRecord::Reflection::ReflectionNameAlreadyTaken do
+      class << firm
+        has_one :quote
+      end
+    end
+  end
-end
-
-
@@ -736 +745 @@
-    assert_equal 2, firm.clients.length
-    assert firm.clients.include?(companies(:second_client))
-  end
+
+  def test_prevent_naming_association_with_existing_name
+    firm = Firm.new
+    assert_raise ActiveRecord::Reflection::ReflectionNameAlreadyTaken do
+      class << firm
+        has_many :methods
+      end
+    end
+  end
-end
-
-class BelongsToAssociationsTest < Test::Unit::TestCase
@@ -992 +1010 @@
-    # the author id of the post should be the id we set
-    assert_equal post.author_id, author2.id
-  end
-
+
+
+
+
+
+
+
+
+
-end
-
-
@@ -1407 +1433 @@
-    assert_equal 1, developer.connection.select_one("SELECT count(*) FROM developers_projects WHERE 
-      project_id = #{project.id} AND developer_id = #{developer.id}")["count(*)"].to_i
-  end
+
+  def test_prevent_naming_association_with_existing_name
+    firm = Firm.new
+    assert_raise ActiveRecord::Reflection::ReflectionNameAlreadyTaken do
+      class << firm
+        has_and_belongs_to_many :methods
+      end
+    end
+  end
-end

activerecord/test/aggregations_test.rb

@@ -44 +44 @@
-    assert_equal "39", customers(:david).gps_location.latitude
-    assert_equal "-110", customers(:david).gps_location.longitude
-  end
+
+  def test_prevent_naming_aggregation_with_existing_name
+    assert_raise ActiveRecord::Reflection::ReflectionNameAlreadyTaken do
+      class << customers(:david)
+        composed_of :methods, :class_name => 'Money'
+      end
+    end
+  end
-end

activerecord/lib/active_record/reflection.rb

@@ -1 +1 @@
-module ActiveRecord
-  module Reflection # :nodoc:
+    class ReflectionNameAlreadyTaken < ActiveRecord::ConfigurationError; end
+
-    def self.included(base)
-      base.extend(ClassMethods)
-    end
@@ -11 +13 @@
-    # You can find the interface for the AggregateReflection and AssociationReflection classes in the abstract MacroReflection class.
-    module ClassMethods
-      def create_reflection(macro, name, options, active_record)
+        guard_against_already_used_method_name(name, active_record)
-        case macro
-          when :has_many, :belongs_to, :has_one, :has_and_belongs_to_many
-            reflections[name] = AssociationReflection.new(macro, name, options, active_record)
@@ -44 +47 @@
-      def reflect_on_association(association)
-        reflections[association].is_a?(AssociationReflection) ? reflections[association] : nil
-      end
+
+      protected
+        def guard_against_already_used_method_name(name, active_record)
+          # We cast a very wide net here, maybe we should not case such as wide one ?
+          methods = [ActiveRecord::Base.instance_methods, ActiveRecord::Base.methods,
+                    active_record.instance_methods].flatten.compact.uniq
+          return unless methods.include?(name.to_s)
+          raise ReflectionNameAlreadyTaken, "`#{name}' is the name of an existing method in #{active_record.name}.  You will have to rename your association."
+        end
-    end
-
-

activerecord/lib/active_record/aggregations.rb

@@ -125 +131 @@
-        class_name  = options[:class_name] || name_to_class_name(name)
-        mapping     = options[:mapping] || [ name, name ]
-
+        create_reflection(:composed_of, part_id, options, self)
+
-        reader_method(name, class_name, mapping)
-        writer_method(name, class_name, mapping)
-        
-        create_reflection(:composed_of, part_id, options, self)
-      end
-
-      private