From 2e9e647fee59d975c9564d96c924d29ffe57f2a2 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Wed, 5 Oct 2011 20:20:04 +0100
Subject: Raise an exception on unknown primary key inside
 AssociationReflection.

An association between two models cannot be made if a relevant key is
unknown, so fail fast rather than generating invalid SQL. Fixes #3207.
---
 activerecord/lib/active_record/errors.rb     | 13 +++++++++++++
 activerecord/lib/active_record/reflection.rb | 12 ++++++++----
 activerecord/test/cases/reflection_test.rb   | 15 +++++++++++++++
 3 files changed, 36 insertions(+), 4 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb
index ad7d8cd63c..96870cb338 100644
--- a/activerecord/lib/active_record/errors.rb
+++ b/activerecord/lib/active_record/errors.rb
@@ -169,4 +169,17 @@ module ActiveRecord
       @errors = errors
     end
   end
+
+  # Raised when a primary key is needed, but there is not one specified in the schema or model.
+  class UnknownPrimaryKey < ActiveRecordError
+    attr_reader :model
+
+    def initialize(model)
+      @model = model
+    end
+
+    def message
+      "Unknown primary key for table #{model.table_name} in model #{model}."
+    end
+  end
 end
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 120ff0cac6..5285060288 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -213,11 +213,11 @@ module ActiveRecord
 
       # klass option is necessary to support loading polymorphic associations
       def association_primary_key(klass = nil)
-        options[:primary_key] || (klass || self.klass).primary_key
+        options[:primary_key] || primary_key(klass || self.klass)
       end
 
       def active_record_primary_key
-        @active_record_primary_key ||= options[:primary_key] || active_record.primary_key
+        @active_record_primary_key ||= options[:primary_key] || primary_key(active_record)
       end
 
       def counter_cache_column
@@ -357,6 +357,10 @@ module ActiveRecord
             active_record.name.foreign_key
           end
         end
+
+        def primary_key(klass)
+          klass.primary_key || raise(UnknownPrimaryKey.new(klass))
+        end
     end
 
     # Holds all the meta-data about a :through association as it was specified
@@ -461,7 +465,7 @@ module ActiveRecord
       # We want to use the klass from this reflection, rather than just delegate straight to
       # the source_reflection, because the source_reflection may be polymorphic. We still
       # need to respect the source_reflection's :primary_key option, though.
-      def association_primary_key(klass = self.klass)
+      def association_primary_key(klass = nil)
         # Get the "actual" source reflection if the immediate source reflection has a
         # source reflection itself
         source_reflection = self.source_reflection
@@ -469,7 +473,7 @@ module ActiveRecord
           source_reflection = source_reflection.source_reflection
         end
 
-        source_reflection.options[:primary_key] || klass.primary_key
+        source_reflection.options[:primary_key] || primary_key(klass || self.klass)
       end
 
       # Gets an array of possible <tt>:through</tt> source reflection names:
diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb
index ca9d88fbd5..69e9fc8d61 100644
--- a/activerecord/test/cases/reflection_test.rb
+++ b/activerecord/test/cases/reflection_test.rb
@@ -18,6 +18,7 @@ require 'models/subscriber'
 require 'models/subscription'
 require 'models/tag'
 require 'models/sponsor'
+require 'models/edge'
 
 class ReflectionTest < ActiveRecord::TestCase
   include ActiveRecord::Reflection
@@ -252,11 +253,25 @@ class ReflectionTest < ActiveRecord::TestCase
     assert_equal "custom_primary_key", Author.reflect_on_association(:tags_with_primary_key).association_primary_key.to_s # nested
   end
 
+  def test_association_primary_key_raises_when_missing_primary_key
+    reflection = ActiveRecord::Reflection::AssociationReflection.new(:fuu, :edge, {}, Author)
+    assert_raises(ActiveRecord::UnknownPrimaryKey) { reflection.association_primary_key }
+
+    through = ActiveRecord::Reflection::ThroughReflection.new(:fuu, :edge, {}, Author)
+    through.stubs(:source_reflection).returns(stub_everything(:options => {}, :class_name => 'Edge'))
+    assert_raises(ActiveRecord::UnknownPrimaryKey) { through.association_primary_key }
+  end
+
   def test_active_record_primary_key
     assert_equal "nick", Subscriber.reflect_on_association(:subscriptions).active_record_primary_key.to_s
     assert_equal "name", Author.reflect_on_association(:essay).active_record_primary_key.to_s
   end
 
+  def test_active_record_primary_key_raises_when_missing_primary_key
+    reflection = ActiveRecord::Reflection::AssociationReflection.new(:fuu, :author, {}, Edge)
+    assert_raises(ActiveRecord::UnknownPrimaryKey) { reflection.active_record_primary_key }
+  end
+
   def test_foreign_type
     assert_equal "sponsorable_type", Sponsor.reflect_on_association(:sponsorable).foreign_type.to_s
     assert_equal "sponsorable_type", Sponsor.reflect_on_association(:thing).foreign_type.to_s
-- 
cgit v1.2.3