raises exception (ActiveRecord::ConfigurationError with message) on habtm association creation if join table contains a primary key

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

7 files changed, 99 insertions, 0 deletions

diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index fd23e59e82..b5d8de2544 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -29,6 +29,10 @@ module ActiveRecord
         end
 
         def insert_record(record, force = true, validate = true)
+          if ActiveRecord::Base.connection.supports_primary_key? && ActiveRecord::Base.connection.primary_key(@reflection.options[:join_table])
+            raise ActiveRecord::ConfigurationError, "Primary key is not allowed in a has_and_belongs_to_many join table (#{@reflection.options[:join_table]})."
+          end
+
           if record.new_record?
             if force
               record.save!
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index c533d4cdb6..fab70f34b9 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -56,6 +56,13 @@ module ActiveRecord
         false
       end
 
+      # Can this adapter determine the primary key for tables not attached
+      # to an ActiveRecord class, such as join tables?  Backend specific, as
+      # the abstract adapter always returns +false+.
+      def supports_primary_key?
+        false
+      end
+
       # Does this adapter support using DISTINCT within COUNT?  This is +true+
       # for all adapters except sqlite.
       def supports_count_distinct?
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index c1f44b51f0..4edb64c2c0 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -208,6 +208,10 @@ module ActiveRecord
         true
       end
       
+      def supports_primary_key? #:nodoc:
+        true
+      end
+
       def supports_savepoints? #:nodoc:
         true
       end
@@ -550,6 +554,12 @@ module ActiveRecord
         keys.length == 1 ? [keys.first, nil] : nil
       end
 
+      # Returns just a table's primary key
+      def primary_key(table)
+        pk_and_sequence = pk_and_sequence_for(table)
+        pk_and_sequence && pk_and_sequence.first
+      end
+
       def case_sensitive_equality_operator
         "= BINARY"
       end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index e77ae93c21..224c007f3c 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -250,6 +250,11 @@ module ActiveRecord
         true
       end
 
+      # Does PostgreSQL support finding primary key on non-ActiveRecord tables?
+      def supports_primary_key? #:nodoc:
+        true
+      end
+
       # Does PostgreSQL support standard conforming strings?
       def supports_standard_conforming_strings?
         # Temporarily set the client message level above error to prevent unintentional
@@ -811,6 +816,12 @@ module ActiveRecord
         nil
       end
 
+      # Returns just a table's primary key
+      def primary_key(table)
+        pk_and_sequence = pk_and_sequence_for(table)
+        pk_and_sequence && pk_and_sequence.first
+      end
+
       # Renames a table.
       def rename_table(name, new_name)
         execute "ALTER TABLE #{quote_table_name(name)} RENAME TO #{quote_table_name(new_name)}"
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index c0f5046bff..0c18c38642 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -101,6 +101,10 @@ module ActiveRecord
         true
       end
 
+      def supports_primary_key? #:nodoc:
+        true
+      end
+
       def requires_reloading?
         true
       end
diff --git a/activerecord/test/cases/associations/habtm_join_table_test.rb b/activerecord/test/cases/associations/habtm_join_table_test.rb
new file mode 100644
index 0000000000..0586df1716
--- /dev/null
+++ b/activerecord/test/cases/associations/habtm_join_table_test.rb
@@ -0,0 +1,45 @@
+require 'cases/helper'
+
+class MyReader < ActiveRecord::Base
+  has_and_belongs_to_many :my_books
+end
+
+class MyBook < ActiveRecord::Base
+  has_and_belongs_to_many :my_readers
+end
+
+class JoinTableTest < ActiveRecord::TestCase
+  def setup
+    ActiveRecord::Base.connection.create_table :my_books, :force => true do |t|
+      t.string :name
+    end
+    assert ActiveRecord::Base.connection.table_exists?(:my_books)
+
+    ActiveRecord::Base.connection.create_table :my_readers, :force => true do |t|
+      t.string :name
+    end
+    assert ActiveRecord::Base.connection.table_exists?(:my_readers)
+
+    ActiveRecord::Base.connection.create_table :my_books_my_readers, :force => true do |t|
+      t.integer :my_book_id
+      t.integer :my_reader_id
+    end
+    assert ActiveRecord::Base.connection.table_exists?(:my_books_my_readers)
+  end
+
+  def teardown
+    ActiveRecord::Base.connection.drop_table :my_books
+    ActiveRecord::Base.connection.drop_table :my_readers
+    ActiveRecord::Base.connection.drop_table :my_books_my_readers
+  end
+
+  uses_transaction :test_should_raise_exception_when_join_table_has_a_primary_key
+  def test_should_raise_exception_when_join_table_has_a_primary_key
+    if ActiveRecord::Base.connection.supports_primary_key?
+      assert_raise ActiveRecord::ConfigurationError do
+        jaime = MyReader.create(:name=>"Jaime")
+        jaime.my_books << MyBook.create(:name=>'Great Expectations')
+      end
+    end
+  end
+end
diff --git a/activerecord/test/cases/pk_test.rb b/activerecord/test/cases/pk_test.rb
index 948a570b93..c121e0aa0f 100644
--- a/activerecord/test/cases/pk_test.rb
+++ b/activerecord/test/cases/pk_test.rb
@@ -98,4 +98,22 @@ class PrimaryKeysTest < ActiveRecord::TestCase
   def test_instance_destroy_should_quote_pkey
     assert_nothing_raised { MixedCaseMonkey.find(1).destroy }
   end
+
+  def test_supports_primary_key
+    assert_nothing_raised NoMethodError do
+      ActiveRecord::Base.connection.supports_primary_key?
+    end
+  end
+
+  def test_primary_key_returns_value_if_it_exists
+    if ActiveRecord::Base.connection.supports_primary_key?
+      assert_equal 'id', ActiveRecord::Base.connection.primary_key('developers')
+    end
+  end
+
+  def test_primary_key_returns_nil_if_it_does_not_exist
+    if ActiveRecord::Base.connection.supports_primary_key?
+      assert_nil ActiveRecord::Base.connection.primary_key('developers_projects')
+    end
+  end
 end