diff --git a/Gemfile b/Gemfile index 32441d56e89..b41ed6d903a 100644 --- a/Gemfile +++ b/Gemfile @@ -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 @@ -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 @@ -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) @@ -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 diff --git a/app/models/vm_reconfigure_task.rb b/app/models/vm_reconfigure_task.rb index de22865c6ef..8c4b07f6401 100644 --- a/app/models/vm_reconfigure_task.rb +++ b/app/models/vm_reconfigure_task.rb @@ -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 diff --git a/config/application.rb b/config/application.rb index 8102a7ccd85..7d217bab6b9 100644 --- a/config/application.rb +++ b/config/application.rb @@ -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 diff --git a/lib/acts_as_ar_model.rb b/lib/acts_as_ar_model.rb index 63eab08d8e7..bee378212ad 100644 --- a/lib/acts_as_ar_model.rb +++ b/lib/acts_as_ar_model.rb @@ -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 diff --git a/lib/extensions/descendant_loader.rb b/lib/extensions/descendant_loader.rb index 2f96473251d..f5ee438ba90 100644 --- a/lib/extensions/descendant_loader.rb +++ b/lib/extensions/descendant_loader.rb @@ -261,8 +261,24 @@ 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 `'", + # "xxx/manageiq/app/models/vm_or_template.rb:6:in `
'", 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 @@ -270,11 +286,48 @@ def descendants 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 `
'", + # + # 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 `'", + # "xxx/manageiq/app/models/vm_or_template.rb:6:in `
'", + # + # 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 diff --git a/lib/miq_preloader.rb b/lib/miq_preloader.rb index 71b260d426c..7c976cadc71 100644 --- a/lib/miq_preloader.rb +++ b/lib/miq_preloader.rb @@ -22,8 +22,9 @@ module MiqPreloader # Currently an array does not work # @return [Array] 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 diff --git a/spec/lib/extensions/ar_to_model_hash_spec.rb b/spec/lib/extensions/ar_to_model_hash_spec.rb index dc0711b7124..8f3725eb429 100644 --- a/spec/lib/extensions/ar_to_model_hash_spec.rb +++ b/spec/lib/extensions/ar_to_model_hash_spec.rb @@ -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 diff --git a/spec/lib/miq_preloader_spec.rb b/spec/lib/miq_preloader_spec.rb index 9d22a8d9a1a..f1342b13cc0 100644 --- a/spec/lib/miq_preloader_spec.rb +++ b/spec/lib/miq_preloader_spec.rb @@ -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 @@ -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 @@ -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 diff --git a/spec/models/vm_reconfigure_task_spec.rb b/spec/models/vm_reconfigure_task_spec.rb index e80c639534c..5d0a81b1f07 100644 --- a/spec/models/vm_reconfigure_task_spec.rb +++ b/spec/models/vm_reconfigure_task_spec.rb @@ -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 @@ -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 diff --git a/spec/support/chargeback_helper.rb b/spec/support/chargeback_helper.rb index fa07d76ca3f..94beca462a0 100644 --- a/spec/support/chargeback_helper.rb +++ b/spec/support/chargeback_helper.rb @@ -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