Skip to content

Commit 4d62cba

Browse files
kbrockkbrock
committed
replace_virtual_field condenses returning includes
Merged_includes via include_to_hash already promotes classes up to a Hash. But there was no code that demoted classes back to an array or symbol / nil. If you look at the changed specs, you can see how this reduces the values. In the rails 7.0 migration, I was going to add special code to introduce this but it seems best to do this at the source
1 parent 939a284 commit 4d62cba

File tree

3 files changed

+52
-38
lines changed

3 files changed

+52
-38
lines changed

‎lib/active_record/virtual_attributes/virtual_fields.rb‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,20 @@ def virtual_field?(name)
3333
end
3434

3535
def replace_virtual_fields(associations)
36-
return associations if associations.blank?
36+
return nil if associations.blank?
3737

38-
case associations
39-
when String, Symbol
40-
virtual_field?(associations) ? replace_virtual_fields(virtual_includes(associations)) : associations.to_sym
41-
when Array
42-
associations.collect { |association| replace_virtual_fields(association) }.compact
43-
when Hash
44-
replace_virtual_field_hash(associations)
45-
else
46-
associations
47-
end
38+
ret =
39+
case associations
40+
when String, Symbol
41+
virtual_field?(associations) ? replace_virtual_fields(virtual_includes(associations)) : associations.to_sym
42+
when Array
43+
associations.filter_map { |association| replace_virtual_fields(association) }
44+
when Hash
45+
replace_virtual_field_hash(associations)
46+
else
47+
associations
48+
end
49+
simplify_includes(ret)
4850
end
4951

5052
def replace_virtual_field_hash(associations)
@@ -98,6 +100,18 @@ def merge_includes(hash1, hash2)
98100
merge_includes(include_to_hash(v1), v2)
99101
end
100102
end
103+
104+
# @param [Hash|Array|Symbol|nil]
105+
def simplify_includes(ret)
106+
case ret
107+
when Hash
108+
ret.size <= 1 && ret.values.first.blank? ? ret.keys.first : ret
109+
when Array
110+
ret.size <= 1 ? ret.first : ret
111+
else
112+
ret
113+
end
114+
end
101115
end
102116
end
103117
end
@@ -267,7 +281,7 @@ def arel_column(field, &block)
267281
end
268282

269283
def construct_join_dependency(associations, join_type) # :nodoc:
270-
associations = klass.replace_virtual_fields(associations)
284+
associations = klass.replace_virtual_fields(associations) || {}
271285
super
272286
end
273287
})

‎spec/virtual_attributes_spec.rb‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,10 @@
127127
it ".replace_virtual_fields" do
128128
expect(TestClass.replace_virtual_fields(:vcol1)).to be_nil
129129
expect(TestClass.replace_virtual_fields(:ref1)).to eq(:ref1)
130-
expect(TestClass.replace_virtual_fields([:vcol1])).to eq([])
131-
expect(TestClass.replace_virtual_fields([:vcol1, :ref1])).to eq([:ref1])
132-
expect(TestClass.replace_virtual_fields(:vcol1 => {})).to eq({})
133-
expect(TestClass.replace_virtual_fields(:vcol1 => {}, :ref1 => {})).to eq(:ref1 => {})
130+
expect(TestClass.replace_virtual_fields([:vcol1].freeze)).to be_nil
131+
expect(TestClass.replace_virtual_fields([:vcol1, :ref1].freeze)).to eq(:ref1)
132+
expect(TestClass.replace_virtual_fields({:vcol1 => {}}.freeze)).to be_nil
133+
expect(TestClass.replace_virtual_fields({:vcol1 => {}, :ref1 => {}}.freeze)).to eq(:ref1)
134134
end
135135
end
136136
end
@@ -158,15 +158,15 @@
158158
end
159159

160160
it ".replace_virtual_fields" do
161-
expect(test_sub_class.replace_virtual_fields(:vcol1)).to be_nil
162-
expect(test_sub_class.replace_virtual_fields(:vcolsub1)).to be_nil
161+
expect(test_sub_class.replace_virtual_fields(:vcol1)).to be_nil
162+
expect(test_sub_class.replace_virtual_fields(:vcolsub1)).to be_nil
163163
expect(test_sub_class.replace_virtual_fields(:ref1)).to eq(:ref1)
164-
expect(test_sub_class.replace_virtual_fields([:vcol1])).to eq([])
165-
expect(test_sub_class.replace_virtual_fields([:vcolsub1])).to eq([])
166-
expect(test_sub_class.replace_virtual_fields([:vcolsub1, :vcol1, :ref1])).to eq([:ref1])
167-
expect(test_sub_class.replace_virtual_fields(:vcol1 => {})).to eq({})
168-
expect(test_sub_class.replace_virtual_fields(:vcolsub1 => {})).to eq({})
169-
expect(test_sub_class.replace_virtual_fields(:vcolsub1 => {}, :vcol1 => {}, :ref1 => {})).to eq(:ref1 => {})
164+
expect(test_sub_class.replace_virtual_fields([:vcol1].freeze)).to be_nil
165+
expect(test_sub_class.replace_virtual_fields([:vcolsub1].freeze)).to be_nil
166+
expect(test_sub_class.replace_virtual_fields([:vcolsub1, :vcol1, :ref1].freeze)).to eq(:ref1)
167+
expect(test_sub_class.replace_virtual_fields({:vcol1 => {}}.freeze)).to be_nil
168+
expect(test_sub_class.replace_virtual_fields({:vcolsub1 => {}}.freeze)).to be_nil
169+
expect(test_sub_class.replace_virtual_fields({:vcolsub1 => {}, :vcol1 => {}, :ref1 => {}}.freeze)).to eq(:ref1)
170170
end
171171
end
172172
end
@@ -341,10 +341,10 @@ def hosts
341341
it ".replace_virtual_fields" do
342342
expect(TestClass.replace_virtual_fields(:vref1)).to be_nil
343343
expect(TestClass.replace_virtual_fields(:ref1)).to eq(:ref1)
344-
expect(TestClass.replace_virtual_fields([:vref1])).to eq([])
345-
expect(TestClass.replace_virtual_fields([:vref1, :ref1])).to eq([:ref1])
346-
expect(TestClass.replace_virtual_fields(:vref1 => {})).to eq({})
347-
expect(TestClass.replace_virtual_fields(:vref1 => {}, :ref1 => {})).to eq(:ref1 => {})
344+
expect(TestClass.replace_virtual_fields([:vref1].freeze)).to be_nil
345+
expect(TestClass.replace_virtual_fields([:vref1, :ref1].freeze)).to eq(:ref1)
346+
expect(TestClass.replace_virtual_fields({:vref1 => {}}.freeze)).to be_nil
347+
expect(TestClass.replace_virtual_fields({:vref1 => {}, :ref1 => {}}.freeze)).to eq(:ref1)
348348
end
349349
end
350350
end
@@ -377,12 +377,12 @@ def hosts
377377
expect(test_sub_class.replace_virtual_fields(:vref1)).to be_nil
378378
expect(test_sub_class.replace_virtual_fields(:vrefsub1)).to be_nil
379379
expect(test_sub_class.replace_virtual_fields(:ref1)).to eq(:ref1)
380-
expect(test_sub_class.replace_virtual_fields([:vref1])).to eq([])
381-
expect(test_sub_class.replace_virtual_fields([:vrefsub1])).to eq([])
382-
expect(test_sub_class.replace_virtual_fields([:vrefsub1, :vref1, :ref1])).to eq([:ref1])
383-
expect(test_sub_class.replace_virtual_fields(:vref1 => {})).to eq({})
384-
expect(test_sub_class.replace_virtual_fields(:vrefsub1 => {})).to eq({})
385-
expect(test_sub_class.replace_virtual_fields(:vrefsub1 => {}, :vref1 => {}, :ref1 => {})).to eq(:ref1 => {})
380+
expect(test_sub_class.replace_virtual_fields([:vref1].freeze)).to be_nil
381+
expect(test_sub_class.replace_virtual_fields([:vrefsub1].freeze)).to be_nil
382+
expect(test_sub_class.replace_virtual_fields([:vrefsub1, :vref1, :ref1].freeze)).to eq(:ref1)
383+
expect(test_sub_class.replace_virtual_fields({:vref1 => {}}.freeze)).to be_nil
384+
expect(test_sub_class.replace_virtual_fields({:vrefsub1 => {}}.freeze)).to be_nil
385+
expect(test_sub_class.replace_virtual_fields({:vrefsub1 => {}, :vref1 => {}, :ref1 => {}}.freeze)).to eq(:ref1)
386386
end
387387
end
388388
end

‎spec/virtual_includes_spec.rb‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -524,17 +524,17 @@
524524
expect(Author.replace_virtual_fields([:book_with_most_bookmarks, :books])).to eq([{:books => :bookmarks}, :books])
525525
expect(Author.replace_virtual_fields(["book_with_most_bookmarks", "books"])).to eq([{:books => :bookmarks}, :books])
526526
expect(Author.replace_virtual_fields([{:book_with_most_bookmarks => {}}, :books])).to eq([{:books => :bookmarks}, :books])
527-
expect(Author.replace_virtual_fields([{:book_with_most_bookmarks => {}}, {:books => {}}])).to eq([{:books => :bookmarks}, {:books => {}}])
527+
expect(Author.replace_virtual_fields([{:book_with_most_bookmarks => {}}, {:books => {}}])).to eq([{:books => :bookmarks}, :books])
528528
end
529529

530530
it "handles hash form of delegates" do
531-
expect(Book.replace_virtual_fields([{:author_name => {}}, {:author_name2 => {}}])).to eq([{:author => {}}, {:author => {}}])
531+
expect(Book.replace_virtual_fields([{:author_name => {}}, {:author_name2 => {}}])).to eq([:author, :author])
532532
end
533533

534534
it "handles non-'includes' virtual_attributes" do
535535
expect(Author.replace_virtual_fields(:nick_or_name)).to eq(nil)
536-
expect(Author.replace_virtual_fields([:nick_or_name])).to eq([])
537-
expect(Author.replace_virtual_fields(:nick_or_name => {})).to eq({})
536+
expect(Author.replace_virtual_fields([:nick_or_name])).to eq(nil)
537+
expect(Author.replace_virtual_fields(:nick_or_name => {})).to eq(nil)
538538
end
539539

540540
it "handles deep includes with va indirect uses(:uses => :books => :bookmarks)" do

0 commit comments

Comments
 (0)