From 11bd634ae4eceda83e4e0be72eaec8169c8396b3 Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Fri, 25 Sep 2020 17:25:02 -0400 Subject: [PATCH] Revert "Refactor uncastable through reflection test to detect join key overrides" --- activerecord/CHANGELOG.md | 4 - activerecord/lib/active_record/reflection.rb | 24 +----- activerecord/test/cases/reflection_test.rb | 82 -------------------- activerecord/test/models/dashboard.rb | 2 - activerecord/test/models/subscriber.rb | 1 - activerecord/test/models/subscription.rb | 1 - 6 files changed, 1 insertion(+), 113 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index ec8ed22d93c..5dace15dc20 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -914,10 +914,6 @@ *Ryuta Kamizono* -* Ensure custom PK types are casted in through reflection queries. - - *Gannon McGibbon* - * Preserve user supplied joins order as much as possible. Fixes #36761, #34328, #24281, #12953. diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 151c93ed48a..c57ac159f1d 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -766,9 +766,7 @@ module ActiveRecord end def klass - @klass ||= delegate_reflection.compute_class(class_name).tap do |klass| - check_reflection_validity!(klass) - end + @klass ||= delegate_reflection.compute_class(class_name) end # Returns the source of the through reflection. It checks both a singularized @@ -999,26 +997,6 @@ module ActiveRecord options[:source_type] || source_reflection.class_name end - def custom_join_key_type? - source_reflection.active_record.attributes_to_define_after_schema_loads.key?( - delegate_reflection.foreign_key - ) - end - - def check_reflection_validity!(klass) - return unless custom_join_key_type? - - through_reflection = options[:through].to_s - - unless klass.reflections.key?(through_reflection) || - klass.reflections.key?(through_reflection.pluralize) - raise NotImplementedError, <<~MSG.squish - In order to correctly type cast #{active_record}.#{active_record.primary_key}, - #{klass} needs to define a :#{through_reflection} association. - MSG - end - end - delegate_methods = AssociationReflection.public_instance_methods - public_instance_methods diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 04007e6a845..9c7e6915bf5 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -518,85 +518,3 @@ class ReflectionTest < ActiveRecord::TestCase end end end - -class UncastableOverriddenAttributeReflectionTest < ActiveRecord::TestCase - class NickType < ActiveRecord::Type::Binary - def serialize(value) - super("nickname-#{value}") - end - - def deserialize(value) - super(value).to_s.delete_prefix("nickname-") - end - - def cast_value(value) - value.to_s - end - end - - class Book < ActiveRecord::Base - end - - class Subscription < ActiveRecord::Base - belongs_to :subscriber - belongs_to :book - attribute :subscriber_id, NickType.new - end - - class Subscriber < ActiveRecord::Base - self.primary_key = "nick" - attribute :nick, NickType.new - has_many :subscriptions - has_one :subscription - has_many :books, through: :subscriptions - has_one :book, through: :subscription - end - - setup do - @subscriber = Subscriber.create!(nick: "unique") - end - - teardown do - Book._reflections.clear - Book.clear_reflections_cache - silence_warnings do - Subscriber.has_many :books, through: :subscriptions - Subscriber.has_one :book, through: :subscription - end - end - - test "uncastable has_many through: reflection" do - error = assert_raises(NotImplementedError) { @subscriber.books } - assert_equal <<~MSG.squish, error.message - In order to correctly type cast #{self.class}::Subscriber.nick, - #{self.class}::Book needs to define a :subscriptions association. - MSG - end - - test "uncastable has_one through: reflection" do - error = assert_raises(NotImplementedError) { @subscriber.book } - assert_equal <<~MSG.squish, error.message - In order to correctly type cast #{self.class}::Subscriber.nick, - #{self.class}::Book needs to define a :subscription association. - MSG - end - - test "fixing uncastable has_many through: reflection with has_many" do - silence_warnings do - Book.has_many :subscriptions - end - @subscriber.books - end - - test "fixing uncastable has_one through: reflection with has_many" do - silence_warnings do - Book.has_many :subscriptions - end - @subscriber.book - end - - test "fixing uncastable has_one through: reflection with has_one" do - Book.has_one :subscription - @subscriber.book - end -end diff --git a/activerecord/test/models/dashboard.rb b/activerecord/test/models/dashboard.rb index ca73c54b41f..d25ceeafb1e 100644 --- a/activerecord/test/models/dashboard.rb +++ b/activerecord/test/models/dashboard.rb @@ -2,6 +2,4 @@ class Dashboard < ActiveRecord::Base self.primary_key = :dashboard_id - - has_one :speedometer end diff --git a/activerecord/test/models/subscriber.rb b/activerecord/test/models/subscriber.rb index e27a45a5385..b21969ca2d6 100644 --- a/activerecord/test/models/subscriber.rb +++ b/activerecord/test/models/subscriber.rb @@ -4,7 +4,6 @@ class Subscriber < ActiveRecord::Base self.primary_key = "nick" has_many :subscriptions has_many :books, through: :subscriptions - has_many :published_books, through: :subscriptions end class SpecialSubscriber < Subscriber diff --git a/activerecord/test/models/subscription.rb b/activerecord/test/models/subscription.rb index 098546aa005..f87315fcd19 100644 --- a/activerecord/test/models/subscription.rb +++ b/activerecord/test/models/subscription.rb @@ -2,7 +2,6 @@ class Subscription < ActiveRecord::Base belongs_to :subscriber, counter_cache: :books_count - belongs_to :published_book belongs_to :book validates_presence_of :subscriber_id, :book_id