Skip to content

Commit 2e9e647

Browse files
committed
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 rails#3207.
1 parent 6474765 commit 2e9e647

File tree

3 files changed

+36
-4
lines changed

3 files changed

+36
-4
lines changed

activerecord/lib/active_record/errors.rb

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,4 +169,17 @@ def initialize(errors)
169169
@errors = errors
170170
end
171171
end
172+
173+
# Raised when a primary key is needed, but there is not one specified in the schema or model.
174+
class UnknownPrimaryKey < ActiveRecordError
175+
attr_reader :model
176+
177+
def initialize(model)
178+
@model = model
179+
end
180+
181+
def message
182+
"Unknown primary key for table #{model.table_name} in model #{model}."
183+
end
184+
end
172185
end

activerecord/lib/active_record/reflection.rb

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,11 +213,11 @@ def association_foreign_key
213213

214214
# klass option is necessary to support loading polymorphic associations
215215
def association_primary_key(klass = nil)
216-
options[:primary_key] || (klass || self.klass).primary_key
216+
options[:primary_key] || primary_key(klass || self.klass)
217217
end
218218

219219
def active_record_primary_key
220-
@active_record_primary_key ||= options[:primary_key] || active_record.primary_key
220+
@active_record_primary_key ||= options[:primary_key] || primary_key(active_record)
221221
end
222222

223223
def counter_cache_column
@@ -357,6 +357,10 @@ def derive_foreign_key
357357
active_record.name.foreign_key
358358
end
359359
end
360+
361+
def primary_key(klass)
362+
klass.primary_key || raise(UnknownPrimaryKey.new(klass))
363+
end
360364
end
361365

362366
# Holds all the meta-data about a :through association as it was specified
@@ -461,15 +465,15 @@ def nested?
461465
# We want to use the klass from this reflection, rather than just delegate straight to
462466
# the source_reflection, because the source_reflection may be polymorphic. We still
463467
# need to respect the source_reflection's :primary_key option, though.
464-
def association_primary_key(klass = self.klass)
468+
def association_primary_key(klass = nil)
465469
# Get the "actual" source reflection if the immediate source reflection has a
466470
# source reflection itself
467471
source_reflection = self.source_reflection
468472
while source_reflection.source_reflection
469473
source_reflection = source_reflection.source_reflection
470474
end
471475

472-
source_reflection.options[:primary_key] || klass.primary_key
476+
source_reflection.options[:primary_key] || primary_key(klass || self.klass)
473477
end
474478

475479
# Gets an array of possible <tt>:through</tt> source reflection names:

activerecord/test/cases/reflection_test.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
require 'models/subscription'
1919
require 'models/tag'
2020
require 'models/sponsor'
21+
require 'models/edge'
2122

2223
class ReflectionTest < ActiveRecord::TestCase
2324
include ActiveRecord::Reflection
@@ -252,11 +253,25 @@ def test_association_primary_key
252253
assert_equal "custom_primary_key", Author.reflect_on_association(:tags_with_primary_key).association_primary_key.to_s # nested
253254
end
254255

256+
def test_association_primary_key_raises_when_missing_primary_key
257+
reflection = ActiveRecord::Reflection::AssociationReflection.new(:fuu, :edge, {}, Author)
258+
assert_raises(ActiveRecord::UnknownPrimaryKey) { reflection.association_primary_key }
259+
260+
through = ActiveRecord::Reflection::ThroughReflection.new(:fuu, :edge, {}, Author)
261+
through.stubs(:source_reflection).returns(stub_everything(:options => {}, :class_name => 'Edge'))
262+
assert_raises(ActiveRecord::UnknownPrimaryKey) { through.association_primary_key }
263+
end
264+
255265
def test_active_record_primary_key
256266
assert_equal "nick", Subscriber.reflect_on_association(:subscriptions).active_record_primary_key.to_s
257267
assert_equal "name", Author.reflect_on_association(:essay).active_record_primary_key.to_s
258268
end
259269

270+
def test_active_record_primary_key_raises_when_missing_primary_key
271+
reflection = ActiveRecord::Reflection::AssociationReflection.new(:fuu, :author, {}, Edge)
272+
assert_raises(ActiveRecord::UnknownPrimaryKey) { reflection.active_record_primary_key }
273+
end
274+
260275
def test_foreign_type
261276
assert_equal "sponsorable_type", Sponsor.reflect_on_association(:sponsorable).foreign_type.to_s
262277
assert_equal "sponsorable_type", Sponsor.reflect_on_association(:thing).foreign_type.to_s

0 commit comments

Comments
 (0)