Skip to content

Commit

Permalink
Use Rails 7 interface for the Preloader
Browse files Browse the repository at this point in the history
See rails commit: e3b9779cb701c63012bc1af007c71dc5a888d35a

Very similar to the change found in:
https://github.com/ManageIQ/activerecord-virtual_attributes/pull/133/files

Stub/mock the preloader in a rails 7 compatible way

Rails 7 preloader requires a scope relation, not an array

Note, it looks like only singular associations for already
available_records are preloaded with an optimization in rails 7.

See: https://www.github.com/rails/rails/pull/42654
  • Loading branch information
jrafanie committed Aug 1, 2024
1 parent 239f128 commit 8de12b6
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 18 deletions.
5 changes: 3 additions & 2 deletions lib/miq_preloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ module MiqPreloader
# Currently an array does not work
# @return [Array<ActiveRecord::Base>] records
def self.preload(records, associations, preload_scope = nil)
preloader = ActiveRecord::Associations::Preloader.new
preloader.preload(records, associations, preload_scope)
# Rails 7 changed the interface. See rails commit: e3b9779cb701c63012bc1af007c71dc5a888d35a
# Note, added Array(records) as it could be a single element
ActiveRecord::Associations::Preloader.new(records: Array(records), associations: associations, available_records: Array(preload_scope)).call
end

# for a record, cache results. Also cache the children's links back
Expand Down
8 changes: 4 additions & 4 deletions spec/lib/extensions/ar_to_model_hash_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@

# we're testing the preload of associations, skip the recursive .to_model_hash
allow_any_instance_of(test_vm_class).to receive(:to_model_hash_recursive)
allow(ActiveRecord::Associations::Preloader).to receive(:new).and_return(mocked_preloader)
end

def assert_preloaded(associations)
expect(mocked_preloader).to receive(:preload) do |_recs, assocs|
expect(assocs).to match_array(associations)
end
allow(ActiveRecord::Associations::Preloader).to receive(:new)
.with(records: kind_of(Array), associations: array_including(associations), available_records: kind_of(Array))
.and_return(mocked_preloader)
expect(mocked_preloader).to receive(:call)

test_vm_class.new.to_model_hash(fixed_options)
end
Expand Down
47 changes: 35 additions & 12 deletions spec/lib/miq_preloader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@
FactoryBot.create_list(:vm, 2, :ext_management_system => ems)
emses = ExtManagementSystem.all.load
vms = Vm.where(:ems_id => emses.select(:id))
expect { preload(emses, :vms, vms) }.to make_database_queries(:count => 1)

# there is now two queries - rails 7, why?
expect { preload(emses, :vms, vms) }.to make_database_queries(:count => 2)
expect { expect(emses.first.vms.size).to eq(2) }.not_to make_database_queries
end

Expand All @@ -55,45 +56,67 @@
vms = Vm.where(:ems_id => ems.id)
vms2 = vms.order(:id).to_a # preloaded vms to be used in tests

# there is a query for every ems
expect { preload(ems, :vms, vms) }.to make_database_queries(:count => 1)
# there is now two queries - rails 7, why?
expect { preload(ems, :vms, vms) }.to make_database_queries(:count => 2)

expect { preload(ems, :vms, vms) }.not_to make_database_queries
expect { expect(ems.vms).to match_array(vms2) }.not_to make_database_queries
# vms does not get loaded, so it is not preloaded
expect { expect(vms.first.ext_management_system).to eq(ems) }.to make_database_queries(:count => 2)
expect { expect(vms.first.ext_management_system).to eq(ems) }.to make_database_queries(:count => 1)
end

it "preloads with a loaded relation (records is a relation)" do
ems
emses = ExtManagementSystem.all.load
vms = Vm.where(:ems_id => emses.select(:id)).load
expect { preload(emses, :vms, vms) }.not_to make_database_queries

# there is now two queries - rails 7, why?
expect { preload(emses, :vms, vms) }.to make_database_queries(:count => 1)

expect { preload(emses, :vms, vms) }.not_to make_database_queries
expect { expect(emses.first.vms.size).to eq(2) }.not_to make_database_queries
expect { expect(vms.first.ext_management_system).to eq(ems) }.not_to make_database_queries

# This PR seems to indicate only the singular associations are preloaded after the rails 7 refactor
# https://github.com/rails/rails/pull/42654. Hence, the next line fails with a single query.
# expect { expect(vms.first.ext_management_system).to eq(ems) }.not_to make_database_queries
end

it "preloads singular association with a loaded relation (records is a relation)" do
emses = ExtManagementSystem.select(:id).where(:id => ems.id).load
vms = Vm.where(:ems_id => emses.select(:id)).load
preload(vms, :ext_management_system, emses)

expect { preload(vms, :ext_management_system, emses) }.not_to make_database_queries
expect { expect(vms.size).to eq(2) }.not_to make_database_queries
expect { expect(vms.first.ext_management_system).to eq(emses.first) }.not_to make_database_queries
end

it "preloads with an array (records is a relation)" do
ems
emses = ExtManagementSystem.all.load
vms = Vm.where(:ems_id => emses.select(:id)).to_a
expect { preload(emses, :vms, vms) }.not_to make_database_queries
expect { preload(emses, :vms, vms) }.to make_database_queries(:count => 1)

expect { preload(emses, :vms, vms) }.not_to make_database_queries
expect { expect(emses.first.vms.size).to eq(2) }.not_to make_database_queries
expect { expect(vms.first.ext_management_system).to eq(ems) }.not_to make_database_queries

# This PR seems to indicate only the singular associations are preloaded after the rails 7 refactor
# https://github.com/rails/rails/pull/42654. Hence, the next line fails with a single query.
# expect { expect(vms.first.ext_management_system).to eq(ems) }.not_to make_database_queries
end

it "preloads with an array (records is an array)" do
ems
emses = ExtManagementSystem.all.load.to_a
vms = Vm.where(:ems_id => emses.map(&:id)).to_a
expect { preload(emses, :vms, vms) }.not_to make_database_queries
expect { preload(emses, :vms, vms) }.to make_database_queries(:count => 1)

expect { preload(emses, :vms, vms) }.not_to make_database_queries
expect { expect(emses.first.vms.size).to eq(2) }.not_to make_database_queries
expect { expect(vms.first.ext_management_system).to eq(ems) }.not_to make_database_queries

# This PR seems to indicate only the singular associations are preloaded after the rails 7 refactor
# https://github.com/rails/rails/pull/42654. Hence, the next line fails with a single query.
# expect { expect(vms.first.ext_management_system).to eq(ems) }.not_to make_database_queries
end

it "preloads a through with a loaded scope" do
Expand All @@ -109,8 +132,8 @@
expect { expect(nodes.first.container_images).to eq([image]) }.to make_database_queries(:count => 1)
end

def preload(*args)
MiqPreloader.preload(*args)
def preload(*args, **kwargs)
MiqPreloader.preload(*args, **kwargs)
end
end

Expand Down

0 comments on commit 8de12b6

Please sign in to comment.