Skip to content

Commit

Permalink
Fixes #37825 - Upgrade Rails to 7.0
Browse files Browse the repository at this point in the history
Refs #37825 - Make reloading work

Refs #37825 - Stop breaking Rails

Refs #37825 - Fix resource nesting for PreloadScopesBuilder

Without this patch Ansible-related scopes for Host::Managed.includes()
would look like:
[ { host_ansible_roles: :ansible_roles }, { ansible_role: [{ ansible_variables:
[:lookup_values] }] } ], which for some reason worked before Rails 7.
After this patch, it looks like:
[ { host_ansible_roles: { ansible_role: [{ ansible_variables:
[:lookup_values] }] } } ].

Refs #37825 - Clear all connections after webpack:compile

Refs #37825 - Enable bootsnap for test, disable reloading for integrational

Refs #37825 - Use #add instead of #<< for ActiveModel::Errors

Refs #37825 - Update comments in config/boot

Refs #37825 - Clear as_deprecation_whitelist.yaml
  • Loading branch information
ofedoren committed Dec 10, 2024
1 parent 2620245 commit c2c9c0e
Show file tree
Hide file tree
Showing 30 changed files with 55 additions and 50 deletions.
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ FOREMAN_GEMFILE = __FILE__ unless defined? FOREMAN_GEMFILE

source 'https://rubygems.org'

gem 'rails', '~> 6.1.6'
gem 'rails', '~> 7.0.3'
gem 'rest-client', '>= 2.0.0', '< 3', :require => 'rest_client'
gem 'audited', '~> 5.0', '!= 5.1.0'
gem 'will_paginate', '~> 3.3'
Expand Down
4 changes: 0 additions & 4 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,6 @@ def process_ajax_error(exception, action = nil)
render :partial => "common/ajax_error", :status => :internal_server_error, :locals => { :message => message }
end

def redirect_back_or_to(url)
redirect_back(fallback_location: url, allow_other_host: false)
end

def saved_redirect_url_or(default)
session["redirect_to_url_#{controller_name}"] || default
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/links_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class LinksController < ApplicationController

def show
url = external_url(type: params[:type], options: params)
redirect_to(url)
redirect_to(url, allow_other_host: true)
end

def external_url(type:, options: {})
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/usergroups_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def update
process_error
end
rescue Foreman::CyclicGraphException => e
@usergroup.errors[:usergroups] << e.record.errors[:base].join(' ')
@usergroup.errors.add(:usergroups, e.record.errors[:base].join(' '))
process_error
rescue => e
external_usergroups_error(@usergroup, e)
Expand Down
2 changes: 1 addition & 1 deletion app/graphql/collection_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def validate
end

def preloader_for_association(records)
::ActiveRecord::Associations::Preloader.new.preload(records, association_name, base_scope).first
::ActiveRecord::Associations::Preloader.new(records: records, associations: association_name, scope: base_scope).call.first
end

def read_association(preloader, record)
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/layout_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def popover(title, msg, options = {})

def icon_text(i, text = "", opts = {})
opts[:kind] ||= "glyphicon"
(content_tag(:span, "", :class => "#{opts[:kind] + ' ' + opts[:kind]}-#{i} #{opts[:class]}", :title => opts[:title]) + " " + text).html_safe
(content_tag(:span, "", :class => "#{opts[:kind] + ' ' + opts[:kind]}-#{i} #{opts[:class]}", :title => opts[:title]) + " " + text.to_s).html_safe
end

def alert(opts = {})
Expand Down
4 changes: 2 additions & 2 deletions app/models/compute_resources/foreman/model/ec2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,9 @@ def test_connection(options = {})
super
errors[:user].empty? && errors[:password].empty? && regions
rescue Fog::AWS::Compute::Error => e
errors[:base] << e.message
errors.add(:base, e.message)
rescue Excon::Error::Socket => e
errors[:base] << e.message
errors.add(:base, e.message)
end

def console(uuid)
Expand Down
2 changes: 1 addition & 1 deletion app/models/compute_resources/foreman/model/libvirt.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def test_connection(options = {})
errors[:url].empty? && hypervisor
rescue => e
disconnect rescue nil
errors[:base] << e.message
errors.add(:base, e.message)
end

def new_nic(attr = {})
Expand Down
2 changes: 1 addition & 1 deletion app/models/compute_resources/foreman/model/openstack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def test_connection(options = {})
super
errors[:user].empty? && errors[:password] && tenants
rescue => e
errors[:base] << e.message
errors.add(:base, e.message)
end

def available_images
Expand Down
2 changes: 1 addition & 1 deletion app/models/compute_resources/foreman/model/vmware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ def test_connection(options = {})
errors.delete(:datacenter)
end
rescue => e
errors[:base] << e.message
errors.add(:base, e.message)
end

def parse_args(args)
Expand Down
8 changes: 0 additions & 8 deletions app/models/concerns/foreman/sti.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,5 @@ def new(attributes = nil, &block)
super
end
end

def save(*args, **kwargs)
type_changed = type_changed?
self.class.instance_variable_set("@finder_needs_type_condition", :false) if type_changed
super
ensure
self.class.instance_variable_set("@finder_needs_type_condition", :true) if type_changed
end
end
end
4 changes: 4 additions & 0 deletions app/models/concerns/hidden_value.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,8 @@ def safe_value
def hidden_value
HIDDEN_VALUE
end

def hidden_value?
attributes['hidden_value']
end
end
2 changes: 1 addition & 1 deletion app/models/concerns/hostext/token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module Token
included do
has_one :token, :foreign_key => :host_id, :dependent => :destroy, :inverse_of => :host, :class_name => 'Token::Build'

scope :for_token, ->(token) { joins(:token).where(:tokens => { :value => token }).where("expires >= ?", Time.now.utc.to_s(:db)).select('hosts.*') }
scope :for_token, ->(token) { joins(:token).where(:tokens => { :value => token }).where("expires >= ?", Time.now.utc.to_formatted_s(:db)).select('hosts.*') }
scope :for_token_when_built, ->(token) { joins(:token).where(:tokens => { :value => token }).select('hosts.*') }

before_validation :refresh_token_on_build
Expand Down
5 changes: 5 additions & 0 deletions app/models/host.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,9 @@ def self.respond_to_missing?(method, include_private = false)
method.to_s =~ /\Afind_by_(.*)\Z/ || method.to_s.include?('create') ||
[:reorder, :new].include?(method) || super
end

# This is a workaround for https://github.com/rails/rails/blob/v7.0.4/activerecord/lib/active_record/reflection.rb#L420-L443
def self.<(other)
other == ActiveRecord::Base || super
end
end
16 changes: 11 additions & 5 deletions app/services/foreman/preload_scopes_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,22 @@ def preload_scopes
end

def build_scopes(model, ignore: [], assoc_name: nil)
scopes = dependent_associations(model).map do |assoc|
scopes = dependent_associations(model, ignore: ignore).map do |assoc|
next if ignore.include?(assoc.name)

dep_associations = dependent_associations(assoc.klass)
if dep_associations.any?
ignore += dep_associations.select { |to_ignore| to_ignore.options.key?(:through) }.map(&:name)
ignore << assoc.name
dep_scopes = build_scopes(assoc.klass, ignore: ignore, assoc_name: assoc.name)
if assoc.options.key?(:through) && dep_scopes.is_a?(Hash)
next { assoc.options[:through] => assoc.source_reflection_name }.merge(dep_scopes)
if assoc.options.key?(:through)
deps = dependent_associations(assoc.source_reflection.klass, ignore: ignore)
if deps.any?
dep_scopes = build_scopes(assoc.source_reflection.klass, ignore: ignore, assoc_name: assoc.source_reflection_name)
next { assoc.options[:through] => dep_scopes }
else
next { assoc.options[:through] => assoc.source_reflection_name }
end
end
next dep_scopes
end
Expand All @@ -46,9 +52,9 @@ def build_scopes(model, ignore: [], assoc_name: nil)
scopes.empty? ? assoc_name : { assoc_name => scopes }
end

def dependent_associations(model)
def dependent_associations(model, ignore: [])
model.reflect_on_all_associations.select do |assoc|
(assoc.options.values & [:destroy, :delete_all, :destroy_async]).any?
!ignore.include?(assoc.name) && (assoc.options.values & [:destroy, :delete_all, :destroy_async]).any?
end
end
end
Expand Down
3 changes: 1 addition & 2 deletions bin/spring
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

# This file loads spring without using Bundler, in order to be fast.
# It gets overwritten when you run the `spring binstub` command.

unless defined?(Spring)
if !defined?(Spring) && ARGV.grep(/test\/integration/).empty? # Disable spring for integration test
require 'rubygems'
require 'bundler'

Expand Down
2 changes: 1 addition & 1 deletion bundler.d/development.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

gem 'bullet', '>= 6.1.0'
gem "parallel_tests"
gem 'spring', '>= 1.0', '< 3'
gem 'spring', '~> 4.0'
gem 'benchmark-ips', '>= 2.8.2'
gem 'foreman'
gem('bootsnap', :require => false)
Expand Down
12 changes: 8 additions & 4 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class Foreman::Consoletie < Rails::Railtie

module Foreman
class Application < Rails::Application
config.load_defaults '6.1'
config.load_defaults '7.0'

# Rails 5.0 changed this to true, but a lot of code depends on this
config.active_record.belongs_to_required_by_default = false
Expand All @@ -99,11 +99,13 @@ class Application < Rails::Application
config.active_support.use_authenticated_message_encryption = false
config.action_dispatch.use_authenticated_cookie_encryption = false

# Rails 6.0 changed this to :zeitwerk
config.autoloader = :zeitwerk

# Rails 6.1 changed this to true, but apparently our codebase is not ready for bidirectional associations
config.active_record.has_many_inversing = false

# Rails 7.0 changed this to true
config.active_record.verify_foreign_keys_for_fixtures = false
config.active_record.automatic_scope_inversing = false

# Setup additional routes by loading all routes file from routes directory
Dir["#{Rails.root}/config/routes/**/*.rb"].each do |route_file|
config.paths['config/routes.rb'] << route_file
Expand All @@ -113,6 +115,8 @@ class Application < Rails::Application
# Application configuration should go into files in config/initializers
# -- all .rb files in that directory are automatically loaded.

# Recommended by Rails: https://guides.rubyonrails.org/v7.0/configuring.html#config-add-autoload-paths-to-load-path
config.add_autoload_paths_to_load_path = false
# Autoloading
config.autoload_paths += %W(#{config.root}/app/models/auth_sources)
config.autoload_paths += %W(#{config.root}/app/models/compute_resources)
Expand Down
2 changes: 0 additions & 2 deletions config/as_deprecation_whitelist.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
---
- message: Initialization autoloaded the constants
- message: DatabaseConfig#config will be removed in 6.2.0 in favor of DatabaseConfigurations#configuration_hash which returns a hash with symbol keys
4 changes: 2 additions & 2 deletions config/boot.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

unless File.exist?(File.expand_path('../Gemfile.in', __dir__))
ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__)
# Set up boootsnap on Ruby 2.3+ in development env with Bundler enabled and development group
# Set up boootsnap on Ruby 2.7+ in development and test env with Bundler enabled and development/test group
early_env = ENV["RAILS_ENV"] || ENV["RACK_ENV"] || "development"
require 'active_support/dependencies'
require('bootsnap/setup') if early_env == "development" && File.exist?(ENV['BUNDLE_GEMFILE']) && !Gem::Specification.stubs_for("bootsnap").empty?
require('bootsnap/setup') if %w[development test].include?(early_env) && File.exist?(ENV['BUNDLE_GEMFILE']) && !Gem::Specification.stubs_for("bootsnap").empty?
end
3 changes: 2 additions & 1 deletion config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
# test suite. You never need to work with it otherwise. Remember that
# your test database is "scratch space" for the test suite and is wiped
# and recreated between test runs. Don't rely on the data there!
config.cache_classes = true
# Disable reloading for integration tests
config.cache_classes = ARGV.grep(/test\/integration/).any?

config.eager_load = true

Expand Down
2 changes: 1 addition & 1 deletion config/initializers/core_extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def self.per_page
# Foreman.in_rake? prevents the failure of db:migrate for postgresql
# don't query settings table if in rake
return 20 unless Foreman.settings.ready?
Setting[:entries_per_page] rescue 20
Foreman.settings[:entries_per_page] rescue 20
end

def self.audited(*args)
Expand Down
1 change: 1 addition & 0 deletions lib/tasks/webpack_compile.rake
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ namespace :webpack do
end

sh "npx --max_old_space_size=#{max_old_space_size} webpack --config #{config_file} --bail"
ActiveRecord::Base.clear_all_connections!
end
end
2 changes: 1 addition & 1 deletion test/factories/audit.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
FactoryBot.define do
factory :audit do
sequence(:version) { |n| n.to_s }
auditable_type { "test" }
auditable_type { ApplicationRecord }
action { "update" }

trait :with_diff do
Expand Down
5 changes: 3 additions & 2 deletions test/integration/hostgroup_js_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ class HostgroupJSTest < IntegrationTestWithJavascript
select2 os.ptables.first.name, :from => 'hostgroup_ptable_id'
fill_in 'hostgroup_root_pass', :with => '12345678'
click_button 'Submit'

hostgroup = Hostgroup.where(:name => "myhostgroup1").first
hostgroup = wait_for do
Hostgroup.where(:name => "myhostgroup1").first
end
refute_nil hostgroup
assert_equal os.id, hostgroup.operatingsystem_id
assert page.has_current_path? hostgroups_path
Expand Down
4 changes: 3 additions & 1 deletion test/integration/org_admin_js_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ def test_org_admin_tries_to_create_domain_when_unselect_the_organization
ensure_selected_option_of_multiselect(@org1.name, select_id: 'domain_organization_ids')
page.click_button 'Submit'

created_domain = Domain.unscoped.find_by_name(domain.name)
created_domain = wait_for do
Domain.unscoped.find_by_name(domain.name)
end
# sets the only organization anyway
assert_equal [@org1], created_domain.organizations
end
Expand Down
1 change: 0 additions & 1 deletion test/unit/foreman/access_control_test.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
require 'test_helper'
require 'foreman/access_control'

class AccessControlTest < ActiveSupport::TestCase
test '#path_hash_to_string reads controller and action' do
Expand Down
1 change: 0 additions & 1 deletion test/unit/foreman/cast_test.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
require 'test_helper'
require 'foreman/cast'

class CastTest < ActiveSupport::TestCase
include Foreman::Cast
Expand Down
2 changes: 1 addition & 1 deletion test/unit/has_many_common_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class HasManyCommonTest < ActiveSupport::TestCase

## Test name methods resolve for Plugin AR objects
class ::FakePlugin; end
class ::FakePlugin::FakeModel; end
class ::FakePlugin::FakeModel < ApplicationRecord; end

test "should return plugin associations" do
Host::Managed.class_eval do
Expand Down
2 changes: 0 additions & 2 deletions test/unit/shared/access_permissions_test_base.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
require 'foreman/access_control'

module AccessPermissionsTestBase
extend ActiveSupport::Concern

Expand Down

0 comments on commit c2c9c0e

Please sign in to comment.