Skip to content

Commit 4b69bf3

Browse files
author
Tom Smyth
committed
Added preloading of inverse association
1 parent 797f263 commit 4b69bf3

File tree

3 files changed

+32
-12
lines changed

3 files changed

+32
-12
lines changed

lib/closure_tree/has_closure_tree_root.rb

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,23 @@ def has_closure_tree_root(assoc_name, options = {})
2929
id_hash = {}
3030
parent_col_id = temp_root.class._ct.options[:parent_column_name]
3131

32+
# Lookup inverse belongs_to association reflection on target class.
33+
inverse = temp_root.class.reflections.values.detect do |r|
34+
r.macro == :belongs_to && r.klass == self.class
35+
end
36+
37+
# Fetch all descendants in constant number of queries.
38+
# This is the last query-triggering statement in the method.
3239
temp_root.self_and_descendants.includes(assoc_map).each do |node|
3340
id_hash[node.id] = node
3441
parent_node = id_hash[node[parent_col_id]]
3542

36-
# Preload parent association
43+
# Pre-assign parent association
3744
parent_assoc = node.association(:parent)
3845
parent_assoc.loaded!
3946
parent_assoc.target = parent_node
4047

41-
# Preload children association as empty for now,
48+
# Pre-assign children association as empty for now,
4249
# children will be added in subsequent loop iterations
4350
children_assoc = node.association(:children)
4451
children_assoc.loaded!
@@ -49,6 +56,13 @@ def has_closure_tree_root(assoc_name, options = {})
4956
# Capture the root we're going to use
5057
root = node
5158
end
59+
60+
# Pre-assign inverse association back to this class, if it exists on target class.
61+
if inverse
62+
inverse_assoc = node.association(inverse.name)
63+
inverse_assoc.loaded!
64+
inverse_assoc.target = self
65+
end
5266
end
5367

5468
root

spec/db/models.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ class User < ActiveRecord::Base
5858
:hierarchy_table_name => 'referral_hierarchies'
5959

6060
has_many :contracts, inverse_of: :user
61+
belongs_to :group # Can't use and don't need inverse_of here when using has_closure_tree_root.
6162

6263
def indirect_contracts
6364
Contract.where(:user_id => descendant_ids)

spec/has_closure_tree_root_spec.rb

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
let!(:user4) { User.create!(email: "[email protected]", group_id: group.id) }
1010
let!(:user5) { User.create!(email: "[email protected]", group_id: group.id) }
1111
let!(:user6) { User.create!(email: "[email protected]", group_id: group.id) }
12+
let!(:group_reloaded) { group.class.find(group.id) } # Ensures were starting fresh.
1213

1314
before do
1415
# The tree (contract types in parens)
@@ -37,13 +38,9 @@
3738
context "with basic config" do
3839
let!(:group) { Group.create!(name: "TheGroup") }
3940

40-
before do
41-
group = Group.first # Ensure we're starting fresh
42-
end
43-
4441
it "loads all nodes and associations in a constant number of queries" do
4542
expect do
46-
root = group.root_user_including_tree(contracts: :contract_type)
43+
root = group_reloaded.root_user_including_tree(contracts: :contract_type)
4744
expect(root.children[0].email).to eq "[email protected]"
4845
expect(root.children[0].parent.children[1].email).to eq "[email protected]"
4946
expect(root.children[1].contracts.map(&:contract_type).map(&:name)).to eq %w(Type1 Type2)
@@ -53,9 +50,17 @@
5350
end.to_not exceed_query_limit(4) # Without this feature, this is 15, and scales with number of nodes.
5451
end
5552

53+
it "eager loads inverse association to group" do
54+
expect do
55+
root = group_reloaded.root_user_including_tree
56+
expect(root.group).to eq group
57+
expect(root.children[0].group).to eq group
58+
end.to_not exceed_query_limit(2)
59+
end
60+
5661
it "works if eager load association map is not given" do
5762
expect do
58-
root = group.root_user_including_tree
63+
root = group_reloaded.root_user_including_tree
5964
expect(root.children[0].email).to eq "[email protected]"
6065
expect(root.children[0].parent.children[1].children[0].email).to eq "[email protected]"
6166
end.to_not exceed_query_limit(2)
@@ -74,7 +79,7 @@
7479

7580
it "should error" do
7681
expect do
77-
root = group.root_user_including_tree(contracts: :contract_type)
82+
root = group_reloaded.root_user_including_tree(contracts: :contract_type)
7883
end.to raise_error(ClosureTree::MultipleRootError)
7984
end
8085
end
@@ -84,7 +89,7 @@
8489
let(:group) { Grouping.create!(name: "TheGrouping") }
8590

8691
it "should still work" do
87-
root = group.root_person_including_tree(contracts: :contract_type)
92+
root = group_reloaded.root_person_including_tree(contracts: :contract_type)
8893
expect(root.children[0].email).to eq "[email protected]"
8994
end
9095
end
@@ -94,7 +99,7 @@
9499

95100
it "should error" do
96101
expect do
97-
root = group.root_user_including_tree(contracts: :contract_type)
102+
root = group_reloaded.root_user_including_tree(contracts: :contract_type)
98103
end.to raise_error(NameError)
99104
end
100105
end
@@ -104,7 +109,7 @@
104109

105110
it "should error" do
106111
expect do
107-
root = group.root_user_including_tree(contracts: :contract_type)
112+
root = group_reloaded.root_user_including_tree(contracts: :contract_type)
108113
end.to raise_error(ActiveRecord::StatementInvalid)
109114
end
110115
end

0 commit comments

Comments
 (0)