Avoid stack level too deep in predicate builder

Check if `query` is different from `attributes` before recursively calling `expand_from_hash`.
Updated cases to account for new company and comment records.
This commit is contained in:
Aaron Frase 2021-02-11 11:32:20 -05:00
parent a27567596b
commit 91d583420b
No known key found for this signature in database
GPG Key ID: 4BCA33C501C32A84
15 changed files with 55 additions and 31 deletions

View File

@ -103,7 +103,9 @@ module ActiveRecord
klass ||= AssociationQueryValue
queries = klass.new(associated_table, value).queries.map! do |query|
expand_from_hash(query)
# If the query produced is identical to attributes don't go any deeper.
# Prevents stack level too deep errors when association and foreign_key are identical.
query == attributes ? self[key, value] : expand_from_hash(query)
end
grouping_queries(queries)

View File

@ -22,7 +22,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
assert_equal 3, authors.size
assert_equal 5, authors[0].posts.size
assert_equal 3, authors[1].posts.size
assert_equal 10, authors[0].posts.collect { |post| post.comments.size }.inject(0) { |sum, i| sum + i }
assert_equal 11, authors[0].posts.collect { |post| post.comments.size }.inject(0) { |sum, i| sum + i }
end
def test_eager_association_loading_with_cascaded_two_levels_and_one_level
@ -30,7 +30,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
assert_equal 3, authors.size
assert_equal 5, authors[0].posts.size
assert_equal 3, authors[1].posts.size
assert_equal 10, authors[0].posts.collect { |post| post.comments.size }.inject(0) { |sum, i| sum + i }
assert_equal 11, authors[0].posts.collect { |post| post.comments.size }.inject(0) { |sum, i| sum + i }
assert_equal 1, authors[0].categorizations.size
assert_equal 2, authors[1].categorizations.size
end
@ -38,7 +38,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
def test_eager_association_loading_with_hmt_does_not_table_name_collide_when_joining_associations
authors = Author.joins(:posts).eager_load(:comments).where(posts: { tags_count: 1 }).order(:id).to_a
assert_equal 3, assert_queries(0) { authors.size }
assert_equal 10, assert_queries(0) { authors[0].comments.size }
assert_equal 11, assert_queries(0) { authors[0].comments.size }
end
def test_eager_association_loading_grafts_stashed_associations_to_correct_parent
@ -82,7 +82,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
assert_equal 3, authors.size
assert_equal 5, authors[0].posts.size
assert_equal 3, authors[1].posts.size
assert_equal 10, authors[0].posts.collect { |post| post.comments.size }.inject(0) { |sum, i| sum + i }
assert_equal 11, authors[0].posts.collect { |post| post.comments.size }.inject(0) { |sum, i| sum + i }
end
def test_eager_association_loading_with_cascaded_two_levels_and_self_table_reference
@ -101,7 +101,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
def test_eager_association_loading_with_cascaded_three_levels_by_ping_pong
firms = Firm.all.merge!(includes: { account: { firm: :account } }, order: "companies.id").to_a
assert_equal 2, firms.size
assert_equal 3, firms.size
assert_equal firms.first.account, firms.first.account.firm.account
assert_equal companies(:first_firm).account, assert_queries(0) { firms.first.account.firm.account }
assert_equal companies(:first_firm).account.firm.account, assert_queries(0) { firms.first.account.firm.account }
@ -184,7 +184,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
authors_relation = Author.all.merge!(includes: [:comments, { posts: :categorizations }], order: "authors.id")
authors = authors_relation.to_a
assert_equal 3, authors.size
assert_equal 10, authors[0].comments.size
assert_equal 11, authors[0].comments.size
assert_equal 1, authors[1].comments.size
assert_equal 5, authors[0].posts.size
assert_equal 3, authors[1].posts.size

View File

@ -414,7 +414,7 @@ class EagerAssociationTest < ActiveRecord::TestCase
def test_eager_association_loading_with_belongs_to
comments = Comment.all.merge!(includes: :post).to_a
assert_equal 11, comments.length
assert_equal 12, comments.length
titles = comments.map { |c| c.post.title }
assert_includes titles, posts(:welcome).title
assert_includes titles, posts(:sti_post_and_comments).title
@ -1175,8 +1175,8 @@ class EagerAssociationTest < ActiveRecord::TestCase
posts = assert_queries(2) do
Post.all.merge!(joins: :comments, includes: :author, order: "comments.id DESC").to_a
end
assert_equal posts(:eager_other), posts[1]
assert_equal authors(:mary), assert_no_queries { posts[1].author }
assert_equal posts(:eager_other), posts[2]
assert_equal authors(:mary), assert_no_queries { posts[2].author }
end
def test_eager_loading_with_conditions_on_joined_table_preloads

View File

@ -2125,7 +2125,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
def test_associations_order_should_be_priority_over_throughs_order
original = authors(:david)
expected = [12, 10, 9, 8, 7, 6, 5, 3, 2, 1]
expected = [13, 12, 10, 9, 8, 7, 6, 5, 3, 2, 1]
assert_equal expected, original.comments_desc.map(&:id)
preloaded = Author.includes(:comments_desc).find(original.id)
assert_equal expected, preloaded.comments_desc.map(&:id)
@ -3060,6 +3060,13 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
assert_equal [1, 2], posts.first.comments.map(&:id).sort
end
def test_has_many_association_with_same_foreign_key_name
assert_nothing_raised do
firm = Firm.find(15)
assert_not_nil(firm.comments.first)
end
end
private
def force_signal37_to_load_all_clients_of_firm
companies(:first_firm).clients_of_firm.load_target

View File

@ -432,7 +432,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase
author = Author.all.merge!(where: ["name = ?", "David"], includes: :comments, order: "comments.id").first
SpecialComment.new; VerySpecialComment.new
assert_no_queries do
assert_equal [1, 2, 3, 5, 6, 7, 8, 9, 10, 12], author.comments.collect(&:id)
assert_equal [1, 2, 3, 5, 6, 7, 8, 9, 10, 12, 13], author.comments.collect(&:id)
end
end
@ -537,7 +537,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase
def test_has_many_through_collection_size_doesnt_load_target_if_not_loaded
author = authors(:david)
assert_equal 10, author.comments.size
assert_equal 11, author.comments.size
assert_not_predicate author.comments, :loaded?
end
@ -748,7 +748,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase
sub_sti_post = SubStiPost.create!(title: "test", body: "test", author_id: 1)
new_comment = sub_sti_post.comments.create(body: "test")
assert_equal [9, 10, new_comment.id], authors(:david).sti_post_comments.map(&:id).sort
assert_equal [9, 10, 13, new_comment.id], authors(:david).sti_post_comments.map(&:id).sort
end
def test_has_many_with_pluralize_table_names_false

View File

@ -15,10 +15,10 @@ class LeftOuterJoinAssociationTest < ActiveRecord::TestCase
def test_merging_multiple_left_joins_from_different_associations
count = Author.joins(:posts).merge(Post.left_joins(:comments).merge(Comment.left_joins(:ratings))).count
assert_equal 16, count
assert_equal 17, count
count = Author.left_joins(:posts).merge(Post.left_joins(:comments).merge(Comment.left_joins(:ratings))).count
assert_equal 16, count
assert_equal 17, count
end
def test_construct_finder_sql_applies_aliases_tables_on_association_conditions
@ -37,8 +37,8 @@ class LeftOuterJoinAssociationTest < ActiveRecord::TestCase
end
def test_left_outer_joins_count_is_same_as_size_of_loaded_results
assert_equal 17, Post.left_outer_joins(:comments).to_a.size
assert_equal 17, Post.left_outer_joins(:comments).count
assert_equal 18, Post.left_outer_joins(:comments).to_a.size
assert_equal 18, Post.left_outer_joins(:comments).count
end
def test_merging_left_joins_should_be_left_joins
@ -80,7 +80,7 @@ class LeftOuterJoinAssociationTest < ActiveRecord::TestCase
end
def test_left_outer_joins_with_string_join
assert_equal 16, Author.left_outer_joins(:posts).joins("LEFT OUTER JOIN comments ON comments.post_id = posts.id").count
assert_equal 17, Author.left_outer_joins(:posts).joins("LEFT OUTER JOIN comments ON comments.post_id = posts.id").count
end
def test_left_outer_joins_with_arel_join
@ -89,7 +89,7 @@ class LeftOuterJoinAssociationTest < ActiveRecord::TestCase
constraint = comments[:post_id].eq(posts[:id])
arel_join = comments.create_join(comments, comments.create_on(constraint), Arel::Nodes::OuterJoin)
assert_equal 16, Author.left_outer_joins(:posts).joins(arel_join).count
assert_equal 17, Author.left_outer_joins(:posts).joins(arel_join).count
end
def test_join_conditions_added_to_join_clause

View File

@ -473,7 +473,7 @@ class CalculationsTest < ActiveRecord::TestCase
assert_equal 2, c[nil]
assert_equal 1, c["DEPENDENTFIRM"]
assert_equal 5, c["CLIENT"]
assert_equal 2, c["FIRM"]
assert_equal 3, c["FIRM"]
end
def test_should_calculate_grouped_by_function_with_table_alias
@ -481,7 +481,7 @@ class CalculationsTest < ActiveRecord::TestCase
assert_equal 2, c[nil]
assert_equal 1, c["DEPENDENTFIRM"]
assert_equal 5, c["CLIENT"]
assert_equal 2, c["FIRM"]
assert_equal 3, c["FIRM"]
end
def test_should_not_overshadow_enumerable_sum
@ -616,7 +616,7 @@ class CalculationsTest < ActiveRecord::TestCase
end
def test_should_count_field_of_root_table_with_conflicting_group_by_column
expected = { 1 => 2, 2 => 1, 4 => 5, 5 => 2, 7 => 1 }
expected = { 1 => 2, 2 => 1, 4 => 5, 5 => 3, 7 => 1 }
assert_equal expected, Post.joins(:comments).group(:post_id).count
assert_equal expected, Post.joins(:comments).group("comments.post_id").count
assert_equal expected, Post.joins(:comments).group(:post_id).select("DISTINCT posts.author_id").count(:all)
@ -873,7 +873,7 @@ class CalculationsTest < ActiveRecord::TestCase
end if current_adapter?(:PostgreSQLAdapter)
def test_group_by_with_limit
expected = { "StiPost" => 2, "SpecialPost" => 1 }
expected = { "StiPost" => 3, "SpecialPost" => 1 }
actual = Post.includes(:comments).group(:type).order(type: :desc).limit(2).count("comments.id")
assert_equal expected, actual
end

View File

@ -1058,7 +1058,7 @@ class FinderTest < ActiveRecord::TestCase
end
def test_find_on_association_proxy_conditions
assert_equal [1, 2, 3, 5, 6, 7, 8, 9, 10, 12], Comment.where(post_id: authors(:david).posts).map(&:id).sort
assert_equal [1, 2, 3, 5, 6, 7, 8, 9, 10, 12, 13], Comment.where(post_id: authors(:david).posts).map(&:id).sort
end
def test_find_on_hash_conditions_with_range
@ -1458,8 +1458,8 @@ class FinderTest < ActiveRecord::TestCase
end
def test_select_values
assert_equal ["1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11"], Company.connection.select_values("SELECT id FROM companies ORDER BY id").map!(&:to_s)
assert_equal ["37signals", "Summit", "Microsoft", "Flamboyant Software", "Ex Nihilo", "RailsCore", "Leetsoft", "Jadedpixel", "Odegy", "Ex Nihilo Part Deux", "Apex"], Company.connection.select_values("SELECT name FROM companies ORDER BY id")
assert_equal ["1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "15"], Company.connection.select_values("SELECT id FROM companies ORDER BY id").map!(&:to_s)
assert_equal ["37signals", "Summit", "Microsoft", "Flamboyant Software", "Ex Nihilo", "RailsCore", "Leetsoft", "Jadedpixel", "Odegy", "Ex Nihilo Part Deux", "Apex", "RVshare"], Company.connection.select_values("SELECT name FROM companies ORDER BY id")
end
def test_select_rows

View File

@ -392,8 +392,8 @@ class InheritanceTest < ActiveRecord::TestCase
end
def test_inheritance_condition
assert_equal 11, Company.count
assert_equal 2, Firm.count
assert_equal 12, Company.count
assert_equal 3, Firm.count
assert_equal 5, Client.count
end
@ -429,7 +429,7 @@ class InheritanceTest < ActiveRecord::TestCase
def test_destroy_all_within_inheritance
Client.destroy_all
assert_equal 0, Client.count
assert_equal 2, Firm.count
assert_equal 3, Firm.count
end
def test_alt_destroy_all_within_inheritance

View File

@ -439,7 +439,7 @@ class HasManyScopingTest < ActiveRecord::TestCase
end
def test_forwarding_to_scoped
assert_equal 4, Comment.search_by_type("Comment").size
assert_equal 5, Comment.search_by_type("Comment").size
assert_equal 2, @welcome.comments.search_by_type("Comment").size
end

View File

@ -63,3 +63,10 @@ sub_special_comment:
post_id: 4
body: Sub special comment
type: SubSpecialComment
recursive_association_comment:
id: 13
post_id: 5
body: afrase
type: Comment
company: 15

View File

@ -65,3 +65,8 @@ another_first_firm_client:
client_of: 1
name: Apex
firm_name: 37signals
recursive_association_fk:
id: 15
type: Firm
name: RVshare

View File

@ -14,6 +14,7 @@ class Comment < ActiveRecord::Base
belongs_to :post, counter_cache: true
belongs_to :author, polymorphic: true
belongs_to :resource, polymorphic: true
belongs_to :company, foreign_key: "company"
has_many :ratings

View File

@ -15,6 +15,7 @@ class Company < AbstractCompany
has_many :developers, through: :contracts
has_many :special_contracts, -> { includes(:special_developer).where.not("developers.id": nil) }
has_many :special_developers, through: :special_contracts
has_many :comments, foreign_key: "company"
alias_attribute :new_name, :name
attribute :metadata, :json

View File

@ -226,6 +226,7 @@ ActiveRecord::Schema.define do
t.datetime :updated_at
t.datetime :deleted_at
t.integer :comments
t.integer :company
end
create_table :companies, force: true do |t|