Skip to content

Commit

Permalink
Merge pull request #22873 from jrafanie/rails7
Browse files Browse the repository at this point in the history 
Rails 7
  • Loading branch information
@Fryguy
Fryguy committed Aug 6, 2024
2 parents b533c2c + b85a0c7 commit 6f8fdeb
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 38 deletions.
12 changes: 6 additions & 6 deletions Gemfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ manageiq_plugin "manageiq-schema"

# Unmodified gems
gem "activerecord-session_store", "~>2.0"
gem "activerecord-virtual_attributes", "~>6.1.2"
gem "activerecord-virtual_attributes", "~>7.0.0"
gem "acts_as_tree", "~>2.7" # acts_as_tree needs to be required so that it loads before ancestry
gem "ancestry", "~>4.1.0", :require => false
gem "awesome_spawn", "~>1.6", :require => false
Expand All @@ -48,11 +48,11 @@ gem "inventory_refresh", "~>2.1", :require => false
gem "kubeclient", "~>4.0", :require => false # For scaling pods at runtime
gem "linux_admin", "~>3.0", :require => false
gem "listen", "~>3.2", :require => false
gem "manageiq-api-client", "~>0.3.6", :require => false
gem "manageiq-api-client", "~>0.5.0", :require => false
gem "manageiq-loggers", "~>1.0", ">=1.1.1", :require => false
gem "manageiq-messaging", "~>1.0", ">=1.4.3", :require => false
gem "manageiq-password", "~>1.0", :require => false
gem "manageiq-postgres_ha_admin", "~>3.2", :require => false
gem "manageiq-postgres_ha_admin", "~>3.3", :require => false
gem "manageiq-ssh-util", "~>0.2.0", :require => false
gem "memoist", "~>0.16.0", :require => false
gem "money", "~>6.13.5", :require => false
Expand All @@ -69,8 +69,8 @@ gem "psych", ">=3.1", :require => false #
gem "query_relation", "~>0.1.0", :require => false
gem "rack", ">=2.2.6.4", :require => false
gem "rack-attack", "~>6.5.0", :require => false
gem "rails", "~>6.1.7", ">=6.1.7.8"
gem "rails-i18n", "~>6.x"
gem "rails", "~>7.0.8", ">=7.0.8.4"
gem "rails-i18n", "~>7.x"
gem "rake", ">=12.3.3", :require => false
gem "rest-client", "~>2.1.0", :require => false
gem "ruby_parser", :require => false # Required for i18n string extraction, and DescentdantLoader (via prism)
Expand Down Expand Up @@ -306,7 +306,7 @@ group :test do
gem "brakeman", "~>5.4", :require => false
gem "bundler-audit", :require => false
gem "capybara", "~>2.5.0", :require => false
gem "db-query-matchers", "~>0.10.0"
gem "db-query-matchers", "~>0.11.0"
gem "factory_bot", "~>5.1", :require => false
gem "simplecov", ">=0.21.2", :require => false
gem "timecop", "~>0.9", "!= 0.9.7", :require => false
Expand Down
2 changes: 1 addition & 1 deletion app/models/vm_reconfigure_task.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def self.build_message(options, key, message, modifier = nil)

def self.build_disk_message(options)
if options[:disk_add].present?
disk_sizes = options[:disk_add].collect { |d| d["disk_size_in_mb"].to_i.megabytes.to_s(:human_size) + ", Type: " + d["type"].to_s }
disk_sizes = options[:disk_add].collect { |d| d["disk_size_in_mb"].to_i.megabytes.to_fs(:human_size) + ", Type: " + d["type"].to_s }
"Add Disks: #{options[:disk_add].length} : #{disk_sizes.join(", ")} "
end
end
Expand Down
2 changes: 0 additions & 2 deletions config/application.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,6 @@ class Application < Rails::Application

config.autoload_paths += config.eager_load_paths

config.autoloader = :zeitwerk

# config.load_defaults 6.1
# Disable defaults as ActiveRecord::Base.belongs_to_required_by_default = true causes MiqRegion.seed to fail validation on belongs_to maintenance zone

Expand Down
2 changes: 1 addition & 1 deletion lib/acts_as_ar_model.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ def attribute_for_inspect(attr_name)
if value.kind_of?(String) && value.length > 50
"#{value[0..50]}...".inspect
elsif value.kind_of?(Date) || value.kind_of?(Time)
%("#{value.to_s(:db)}")
%("#{value.to_fs(:db)}")
else
value.inspect
end
Expand Down
63 changes: 58 additions & 5 deletions lib/extensions/descendant_loader.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,20 +261,73 @@ def scoped_name(name, scopes)
end

module ArDescendantsWithLoader
# Use caution if modifying the list of excluded caller_locations below.
# These methods are called very early from rails during code load time,
# before the models are even loaded, as documented below.
# Ideally, a more resilient conditional can be used in the future.
#
# Called from __update_callbacks during model load time:
# "xxx/gems/activesupport-7.0.8.4/lib/active_support/callbacks.rb:706:in `__update_callbacks'",
# "xxx/gems/activesupport-7.0.8.4/lib/active_support/callbacks.rb:764:in `set_callback'",
# "xxx/gems/activemodel-7.0.8.4/lib/active_model/validations.rb:172:in `validate'",
# "xxx/gems/activemodel-7.0.8.4/lib/active_model/validations/with.rb:96:in `block in validates_with'",
# "xxx/gems/activemodel-7.0.8.4/lib/active_model/validations/with.rb:85:in `each'",
# "xxx/gems/activemodel-7.0.8.4/lib/active_model/validations/with.rb:85:in `validates_with'",
# "xxx/gems/activemodel-7.0.8.4/lib/active_model/validations/format.rb:109:in `validates_format_of'",
# "xxx/gems/ancestry-4.1.0/lib/ancestry/has_ancestry.rb:30:in `has_ancestry'",
# "xxx/manageiq/app/models/vm_or_template.rb:15:in `<class:VmOrTemplate>'",
# "xxx/manageiq/app/models/vm_or_template.rb:6:in `<main>'",
def descendants
if Vmdb::Application.instance.initialized? && !defined? @loaded_descendants
if Vmdb::Application.instance.initialized? && !defined?(@loaded_descendants) && %w[__update_callbacks].exclude?(caller_locations(1..1).first.base_label)
@loaded_descendants = true
DescendantLoader.instance.load_subclasses(self)
end

super
end

# Rails 6.1 added an alias for subclasss to call direct_descendants in:
# https://github.com/rails/rails/commit/8f8aa857e084b76b1120edaa9bb9ce03ba1e6a19
# We need to get in front of it, like we do for descendants.
# Use caution if modifying the list of excluded caller_locations below.
# These methods are called very early from rails during code load time,
# before the models are even loaded, as documented below.
# Ideally, a more resilient conditional can be used in the future.
#
# Called from reload_schema_from_cache from virtual column definitions from ar_region from many/all models:
# "xxx/gems/activerecord-7.0.8.4/lib/active_record/model_schema.rb:609:in `reload_schema_from_cache'",
# "xxx/gems/activerecord-7.0.8.4/lib/active_record/timestamp.rb:94:in `reload_schema_from_cache'",
# "xxx/bundler/gems/activerecord-virtual_attributes-2c077434608f/lib/active_record/virtual_attributes.rb:60:in `virtual_attribute'",
# "xxx/bundler/gems/activerecord-virtual_attributes-2c077434608f/lib/active_record/virtual_attributes.rb:55:in `virtual_column'",
# "xxx/manageiq/lib/extensions/ar_region.rb:14:in `block in inherited'",
# "xxx/manageiq/lib/extensions/ar_region.rb:13:in `class_eval'",
# "xxx/manageiq/lib/extensions/ar_region.rb:13:in `inherited'",
# "xxx/manageiq/app/models/vm_or_template.rb:6:in `<main>'",
#
# Called from subclasses call in descendant_loader after the above callstack:
# "xxx/gems/activesupport-7.0.8.4/lib/active_support/descendants_tracker.rb:83:in `subclasses'",
# "xxx/manageiq/lib/extensions/descendant_loader.rb:313:in `subclasses'",
# "xxx/gems/activerecord-7.0.8.4/lib/active_record/model_schema.rb:609:in `reload_schema_from_cache'",
# ...
#
# Called from descendants from descendant_loader via __update_callbacks:
# "xxx/gems/activesupport-7.0.8.4/lib/active_support/descendants_tracker.rb:89:in `descendants'",
# "xxx/manageiq/lib/extensions/descendant_loader.rb:296:in `descendants'",
# "xxx/gems/activesupport-7.0.8.4/lib/active_support/callbacks.rb:706:in `__update_callbacks'",
# "xxx/gems/activesupport-7.0.8.4/lib/active_support/callbacks.rb:764:in `set_callback'",
# "xxx/gems/activemodel-7.0.8.4/lib/active_model/validations.rb:172:in `validate'",
# "xxx/gems/activemodel-7.0.8.4/lib/active_model/validations/with.rb:96:in `block in validates_with'",
# "xxx/gems/activemodel-7.0.8.4/lib/active_model/validations/with.rb:85:in `each'",
# "xxx/gems/activemodel-7.0.8.4/lib/active_model/validations/with.rb:85:in `validates_with'",
# "xxx/gems/activemodel-7.0.8.4/lib/active_model/validations/format.rb:109:in `validates_format_of'",
# "xxx/gems/ancestry-4.1.0/lib/ancestry/has_ancestry.rb:30:in `has_ancestry'",
# "xxx/manageiq/app/models/vm_or_template.rb:15:in `<class:VmOrTemplate>'",
# "xxx/manageiq/app/models/vm_or_template.rb:6:in `<main>'",
#
# Called from subclasses from above callstack:
# "xxx/gems/activesupport-7.0.8.4/lib/active_support/descendants_tracker.rb:83:in `subclasses'",
# "xxx/manageiq/lib/extensions/descendant_loader.rb:313:in `subclasses'",
# "xxx/gems/activesupport-7.0.8.4/lib/active_support/descendants_tracker.rb:89:in `descendants'",
# ...
def subclasses
if Vmdb::Application.instance.initialized? && !defined? @loaded_descendants
if Vmdb::Application.instance.initialized? && !defined?(@loaded_descendants) && %w[descendants reload_schema_from_cache subclasses].exclude?(caller_locations(1..1).first.base_label)
@loaded_descendants = true
DescendantLoader.instance.load_subclasses(self)
end
Expand Down
5 changes: 3 additions & 2 deletions lib/miq_preloader.rb
View file
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
View file
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
57 changes: 44 additions & 13 deletions spec/lib/miq_preloader_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@
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)

# TODO: With rails 7, the query count below increased by 1.
# This PR says only the singular associations are preloaded in rails 7:
# https://github.com/rails/rails/pull/42654
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 +58,73 @@
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)
# TODO: With rails 7, the query count below increased by 1.
# This PR says only the singular associations are preloaded in rails 7:
# https://github.com/rails/rails/pull/42654
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)

# TODO: With rails 7, the query count below decreased by 1.
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

# TODO: With rails 7, the query count below increased by 1.
# This PR says only the singular associations are preloaded in rails 7:
# https://github.com/rails/rails/pull/42654
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

# TODO: With rails 7, the query count below increased by 1.
expect { expect(vms.first.ext_management_system).to eq(ems) }.to make_database_queries(:count => 1)
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

# TODO: With rails 7, the query count below increased by 1.
# This PR says only the singular associations are preloaded in rails 7:
# https://github.com/rails/rails/pull/42654
expect { expect(vms.first.ext_management_system).to eq(ems) }.to make_database_queries(:count => 1)
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

# TODO: With rails 7, the query count below increased by 1.
# This PR says only the singular associations are preloaded in rails 7:
# https://github.com/rails/rails/pull/42654
expect { expect(vms.first.ext_management_system).to eq(ems) }.to make_database_queries(:count => 1)
end

it "preloads a through with a loaded scope" do
Expand All @@ -109,8 +140,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
6 changes: 3 additions & 3 deletions spec/models/vm_reconfigure_task_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
context "Single Disk add " do
let(:request_options) { {:disk_add => [{"disk_size_in_mb" => "33", "persistent" => "true", "type" => "thin"}.with_indifferent_access]} }
let(:description_partial) do
"Add Disks: 1 : #{request.options[:disk_add][0]["disk_size_in_mb"].to_i.megabytes.to_s(:human_size)}, Type: " \
"Add Disks: 1 : #{request.options[:disk_add][0]["disk_size_in_mb"].to_i.megabytes.to_fs(:human_size)}, Type: " \
"#{request.options[:disk_add][0]["type"]} "
end

Expand All @@ -55,8 +55,8 @@
{"disk_size_in_mb" => "44", "persistent" => "true", "type" => "thick"}.with_indifferent_access]}
end
let(:description_partial) do
"Add Disks: 2 : #{request.options[:disk_add][0]["disk_size_in_mb"].to_i.megabytes.to_s(:human_size)}, Type: " \
"#{request.options[:disk_add][0]["type"]}, #{request.options[:disk_add][1]["disk_size_in_mb"].to_i.megabytes.to_s(:human_size)}, Type: " \
"Add Disks: 2 : #{request.options[:disk_add][0]["disk_size_in_mb"].to_i.megabytes.to_fs(:human_size)}, Type: " \
"#{request.options[:disk_add][0]["type"]}, #{request.options[:disk_add][1]["disk_size_in_mb"].to_i.megabytes.to_fs(:human_size)}, Type: " \
"#{request.options[:disk_add][1]["type"]} "
end

Expand Down
2 changes: 1 addition & 1 deletion spec/support/chargeback_helper.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def add_metric_rollups_for(resources, range, step, metric_rollup_params, trait =
base_values
end

record_values << "(#{values}, '#{time.to_s(:db)}')"
record_values << "(#{values}, '#{time.to_fs(:db)}')"
end
end

Expand Down

0 comments on commit 6f8fdeb

Please sign in to comment.