From 2301dddcff8c0efe551c3c3baeeea63dadbe0ea6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= Date: Thu, 20 Jun 2024 10:33:01 +0200 Subject: [PATCH] DEV: Upgrade Rails to version 7.1 (#27539) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * DEV: Upgrade Rails to 7.1 * FIX: Remove references to `Rails.logger.chained` `Rails.logger.chained` was provided by Logster before Rails 7.1 introduced their broadcast logger. Now all the loggers are added to `Rails.logger.broadcasts`. Some code in our initializers was still using `chained` instead of `broadcasts`. * DEV: Make parameters optional to all FakeLogger methods * FIX: Set `override_level` on Logster loggers (#27519) A followup to f595d599dd361b7fb39fb3c82cbc11d19d518c19 * FIX: Don’t duplicate Rack response --------- Co-authored-by: Jarek Radosz --- .licensed.yml | 4 + Gemfile | 14 +- Gemfile.lock | 103 +++++--- app/models/post_revision.rb | 2 +- app/models/reviewable.rb | 6 +- app/models/translation_override.rb | 1 + app/models/user.rb | 4 +- app/services/staff_action_logger.rb | 2 +- config/application.rb | 19 +- config/environments/development.rb | 7 + config/environments/test.rb | 3 +- config/initializers/002-rails_failover.rb | 2 +- config/initializers/100-logster.rb | 11 +- config/initializers/100-quiet_logger.rb | 6 +- config/initializers/100-silence_logger.rb | 13 +- config/initializers/101-lograge.rb | 4 +- config/initializers/102-truncate-logs.rb | 6 +- .../new_framework_defaults_7_0.rb | 106 --------- .../new_framework_defaults_7_1.rb | 224 ++++++++++++++++++ config/locales/server.en.yml | 1 + lib/discourse.rb | 18 +- lib/hijack.rb | 6 +- lib/middleware/discourse_public_exceptions.rb | 2 +- lib/site_setting_extension.rb | 2 +- .../chat/app/models/chat/category_channel.rb | 2 +- .../app/models/chat/direct_message_channel.rb | 2 +- .../chat/spec/system/visit_channel_spec.rb | 2 +- spec/integration/invalid_request_spec.rb | 21 +- .../default_current_user_provider_spec.rb | 6 +- spec/lib/email/message_builder_spec.rb | 8 +- spec/lib/hijack_spec.rb | 10 +- spec/lib/migration/safe_migrate_spec.rb | 7 +- spec/lib/site_setting_extension_spec.rb | 4 +- spec/models/admin_dashboard_data_spec.rb | 2 +- spec/models/user_spec.rb | 2 +- spec/rails_helper.rb | 4 +- spec/requests/application_controller_spec.rb | 2 +- spec/requests/categories_controller_spec.rb | 8 +- spec/requests/embed_controller_spec.rb | 4 +- spec/requests/session_controller_spec.rb | 4 +- .../uploads_controller_multisite_spec.rb | 6 +- spec/support/fake_logger.rb | 19 +- spec/support/i18n_helpers.rb | 3 + spec/support/locales/dashboard.en.yml | 6 + spec/support/locales/message_builder.en.yml | 2 + spec/support/locales/problem_check.en.yml | 2 + 46 files changed, 436 insertions(+), 256 deletions(-) delete mode 100644 config/initializers/new_framework_defaults_7_0.rb create mode 100644 config/initializers/new_framework_defaults_7_1.rb create mode 100644 spec/support/locales/dashboard.en.yml create mode 100644 spec/support/locales/message_builder.en.yml create mode 100644 spec/support/locales/problem_check.en.yml diff --git a/.licensed.yml b/.licensed.yml index c5903c31e0c..314a5e82ac4 100644 --- a/.licensed.yml +++ b/.licensed.yml @@ -31,6 +31,10 @@ ignored: - timeout # Ruby (default gem) - uri # Ruby (default gem) - mutex_m # BSD-2-Clause + - irb # Ruby (default gem) + - rdoc # Ruby (default gem) + - reline # Ruby (default gem) + - stringio # Ruby (default gem) reviewed: bundler: diff --git a/Gemfile b/Gemfile index b053716bfcd..ad2429502ff 100644 --- a/Gemfile +++ b/Gemfile @@ -6,13 +6,13 @@ source "https://rubygems.org" gem "bootsnap", require: false, platform: :mri -gem "actionmailer", "< 7.1" -gem "actionpack", "< 7.1" -gem "actionview", "< 7.1" -gem "activemodel", "< 7.1" -gem "activerecord", "< 7.1" -gem "activesupport", "< 7.1" -gem "railties", "< 7.1" +gem "actionmailer", "~> 7.1.0" +gem "actionpack", "~> 7.1.0" +gem "actionview", "~> 7.1.0" +gem "activemodel", "~> 7.1.0" +gem "activerecord", "~> 7.1.0" +gem "activesupport", "~> 7.1.0" +gem "railties", "~> 7.1.0" gem "sprockets-rails" gem "json" diff --git a/Gemfile.lock b/Gemfile.lock index f55a203b730..6d3729dd268 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,45 +1,54 @@ GEM remote: https://rubygems.org/ specs: - actionmailer (7.0.8.4) - actionpack (= 7.0.8.4) - actionview (= 7.0.8.4) - activejob (= 7.0.8.4) - activesupport (= 7.0.8.4) + actionmailer (7.1.3.4) + actionpack (= 7.1.3.4) + actionview (= 7.1.3.4) + activejob (= 7.1.3.4) + activesupport (= 7.1.3.4) mail (~> 2.5, >= 2.5.4) net-imap net-pop net-smtp - rails-dom-testing (~> 2.0) - actionpack (7.0.8.4) - actionview (= 7.0.8.4) - activesupport (= 7.0.8.4) - rack (~> 2.0, >= 2.2.4) + rails-dom-testing (~> 2.2) + actionpack (7.1.3.4) + actionview (= 7.1.3.4) + activesupport (= 7.1.3.4) + nokogiri (>= 1.8.5) + racc + rack (>= 2.2.4) + rack-session (>= 1.0.1) rack-test (>= 0.6.3) - rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.0, >= 1.2.0) - actionview (7.0.8.4) - activesupport (= 7.0.8.4) + rails-dom-testing (~> 2.2) + rails-html-sanitizer (~> 1.6) + actionview (7.1.3.4) + activesupport (= 7.1.3.4) builder (~> 3.1) - erubi (~> 1.4) - rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.1, >= 1.2.0) + erubi (~> 1.11) + rails-dom-testing (~> 2.2) + rails-html-sanitizer (~> 1.6) actionview_precompiler (0.4.0) actionview (>= 6.0.a) active_model_serializers (0.8.4) activemodel (>= 3.0) - activejob (7.0.8.4) - activesupport (= 7.0.8.4) + activejob (7.1.3.4) + activesupport (= 7.1.3.4) globalid (>= 0.3.6) - activemodel (7.0.8.4) - activesupport (= 7.0.8.4) - activerecord (7.0.8.4) - activemodel (= 7.0.8.4) - activesupport (= 7.0.8.4) - activesupport (7.0.8.4) + activemodel (7.1.3.4) + activesupport (= 7.1.3.4) + activerecord (7.1.3.4) + activemodel (= 7.1.3.4) + activesupport (= 7.1.3.4) + timeout (>= 0.4.0) + activesupport (7.1.3.4) + base64 + bigdecimal concurrent-ruby (~> 1.0, >= 1.0.2) + connection_pool (>= 2.2.5) + drb i18n (>= 1.6, < 2) minitest (>= 5.1) + mutex_m tzinfo (~> 2.0) addressable (2.8.6) public_suffix (>= 2.0.2, < 6.0) @@ -176,6 +185,10 @@ GEM progress (~> 3.0, >= 3.0.1) image_size (3.4.0) in_threads (1.6.0) + io-console (0.7.2) + irb (1.13.1) + rdoc (>= 4.0.0) + reline (>= 0.4.2) iso8601 (0.13.0) jmespath (1.6.2) json (2.7.2) @@ -321,6 +334,8 @@ GEM pry-stack_explorer (0.6.1) binding_of_caller (~> 1.0) pry (~> 0.13) + psych (5.1.2) + stringio public_suffix (5.1.1) puma (6.4.2) nio4r (~> 2.0) @@ -331,8 +346,13 @@ GEM rack-protection (3.2.0) base64 (>= 0.1.0) rack (~> 2.2, >= 2.2.4) + rack-session (1.0.2) + rack (< 3) rack-test (2.1.0) rack (>= 1.3) + rackup (1.0.0) + rack (< 3) + webrick rails-dom-testing (2.2.0) activesupport (>= 5.0.0) minitest @@ -347,13 +367,14 @@ GEM rails_multisite (6.0.0) activerecord (>= 6.0) railties (>= 6.0) - railties (7.0.8.4) - actionpack (= 7.0.8.4) - activesupport (= 7.0.8.4) - method_source + railties (7.1.3.4) + actionpack (= 7.1.3.4) + activesupport (= 7.1.3.4) + irb + rackup (>= 1.0.0) rake (>= 12.2) - thor (~> 1.0) - zeitwerk (~> 2.5) + thor (~> 1.0, >= 1.2.2) + zeitwerk (~> 2.6) rainbow (3.1.1) raindrops (0.20.1) rake (13.2.1) @@ -365,11 +386,15 @@ GEM msgpack (>= 0.4.3) optimist (>= 3.0.0) rchardet (1.8.0) + rdoc (6.7.0) + psych (>= 4.0.0) redcarpet (3.6.0) redis (4.8.1) redis-namespace (1.11.0) redis (>= 4) regexp_parser (2.9.2) + reline (0.5.8) + io-console (~> 0.5) request_store (1.7.0) rack (>= 1.4) rexml (3.3.0) @@ -508,6 +533,7 @@ GEM sqlite3 (1.7.3-x86_64-linux) sshkey (3.0.0) stackprof (0.2.26) + stringio (3.1.0) strscan (3.1.0) syntax_tree (6.2.0) prettier_print (>= 1.2.0) @@ -538,6 +564,7 @@ GEM addressable (>= 2.8.0) crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) + webrick (1.8.1) websocket (1.2.10) xpath (3.2.0) nokogiri (~> 1.8) @@ -559,14 +586,14 @@ PLATFORMS x86_64-linux DEPENDENCIES - actionmailer (< 7.1) - actionpack (< 7.1) - actionview (< 7.1) + actionmailer (~> 7.1.0) + actionpack (~> 7.1.0) + actionview (~> 7.1.0) actionview_precompiler active_model_serializers (~> 0.8.3) - activemodel (< 7.1) - activerecord (< 7.1) - activesupport (< 7.1) + activemodel (~> 7.1.0) + activerecord (~> 7.1.0) + activesupport (~> 7.1.0) addressable annotate aws-sdk-s3 @@ -654,7 +681,7 @@ DEPENDENCIES rails-dom-testing rails_failover rails_multisite - railties (< 7.1) + railties (~> 7.1.0) rake rb-fsevent rbtrace diff --git a/app/models/post_revision.rb b/app/models/post_revision.rb index 7422726fec1..87f8c8d9145 100644 --- a/app/models/post_revision.rb +++ b/app/models/post_revision.rb @@ -4,7 +4,7 @@ class PostRevision < ActiveRecord::Base belongs_to :post belongs_to :user - serialize :modifications, Hash + serialize :modifications, type: Hash after_create :create_notification diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb index 3c03fae8a11..3a0a228d265 100644 --- a/app/models/reviewable.rb +++ b/app/models/reviewable.rb @@ -33,9 +33,13 @@ class Reviewable < ActiveRecord::Base has_many :reviewable_scores, -> { order(created_at: :desc) }, dependent: :destroy enum :status, { pending: 0, approved: 1, rejected: 2, ignored: 3, deleted: 4 } - enum :priority, { low: 0, medium: 5, high: 10 }, scopes: false, suffix: true + + attribute :sensitivity, :integer enum :sensitivity, { disabled: 0, low: 9, medium: 6, high: 3 }, scopes: false, suffix: true + attribute :priority, :integer + enum :priority, { low: 0, medium: 5, high: 10 }, scopes: false, suffix: true + validates :reject_reason, length: { maximum: 2000 } after_create { log_history(:created, created_by) } diff --git a/app/models/translation_override.rb b/app/models/translation_override.rb index 0e891f793ff..d37e7301588 100644 --- a/app/models/translation_override.rb +++ b/app/models/translation_override.rb @@ -45,6 +45,7 @@ class TranslationOverride < ActiveRecord::Base validate :check_interpolation_keys + attribute :status, :integer enum status: { up_to_date: 0, outdated: 1, invalid_interpolation_keys: 2, deprecated: 3 } def self.upsert!(locale, key, value) diff --git a/app/models/user.rb b/app/models/user.rb index d3b2d8bd19e..47464ebc707 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -158,7 +158,9 @@ class User < ActiveRecord::Base unless: :should_skip_user_fields_validation? validates_associated :primary_email, - message: ->(_, user_email) { user_email[:value]&.errors&.[](:email)&.first } + message: ->(_, user_email) do + user_email[:value]&.errors&.[](:email)&.first.to_s + end after_initialize :add_trust_level diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index 7eb767919d8..b52cc2602bc 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -549,7 +549,7 @@ class StaffActionLogger changed_attributes = category.previous_changes.slice(*category_params.keys) - if !old_permissions.empty? && (old_permissions != category_params[:permissions]) + if !old_permissions.empty? && (old_permissions != category_params[:permissions].to_h) changed_attributes.merge!( permissions: [old_permissions.to_json, category_params[:permissions].to_json], ) diff --git a/config/application.rb b/config/application.rb index 2e214e1c5a9..a5f5b728be0 100644 --- a/config/application.rb +++ b/config/application.rb @@ -90,7 +90,7 @@ module Discourse # tiny file needed by site settings require "highlight_js" - config.load_defaults 6.1 + config.load_defaults 7.0 config.active_record.cache_versioning = false # our custom cache class doesn’t support this config.action_controller.forgery_protection_origin_check = false config.active_record.belongs_to_required_by_default = false @@ -100,6 +100,9 @@ module Discourse Time, Symbol, ] + config.active_support.key_generator_hash_digest_class = OpenSSL::Digest::SHA1 + config.action_dispatch.cookies_serializer = :hybrid + config.action_controller.wrap_parameters_by_default = false # we skip it cause we configure it in the initializer # the railtie for message_bus would insert it in the @@ -112,8 +115,9 @@ module Discourse ENV["DISCOURSE_MULTISITE_CONFIG_PATH"] || GlobalSetting.multisite_config_path config.multisite_config_path = File.absolute_path(multisite_config_path, Rails.root) + config.autoload_lib(ignore: %w[common_passwords emoji generators javascripts tasks]) + Rails.autoloaders.main.do_not_eager_load(config.root.join("lib")) # Custom directories with classes and modules you want to be autoloadable. - config.autoload_paths << "#{root}/lib" config.autoload_paths << "#{root}/lib/guardian" config.autoload_paths << "#{root}/lib/i18n" config.autoload_paths << "#{root}/lib/validators" @@ -240,6 +244,17 @@ module Discourse # we got to clear the pool in case plugins connect ActiveRecord::Base.connection_handler.clear_active_connections! + + # Mailers and controllers may have been patched by plugins and when the + # application is eager loaded, the list of public methods is cached. + # We need to invalidate the existing caches, otherwise the new actions + # won’t be seen by Rails. + if Rails.configuration.eager_load + AbstractController::Base.descendants.each do |controller| + controller.clear_action_methods! + controller.action_methods + end + end end require "rbtrace" if ENV["RBTRACE"] == "1" diff --git a/config/environments/development.rb b/config/environments/development.rb index 1ea67794833..6609ecf373b 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -99,4 +99,11 @@ Discourse::Application.configure do end config.hosts << /\A(([a-z0-9-]+)\.)*localhost(\:\d+)?\Z/ + + config.generators.after_generate do |files| + parsable_files = files.filter { |file| file.end_with?(".rb") } + unless parsable_files.empty? + system("bundle exec rubocop -A --fail-level=E #{parsable_files.shelljoin}", exception: true) + end + end end diff --git a/config/environments/test.rb b/config/environments/test.rb index 7f6bd425fdf..66f7e3f32b8 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -26,7 +26,7 @@ Discourse::Application.configure do # production has "show exceptions" on so we better have it # in test as well - config.action_dispatch.show_exceptions = true + config.action_dispatch.show_exceptions = :all # Disable request forgery protection in test environment config.action_controller.allow_forgery_protection = false @@ -65,6 +65,7 @@ Discourse::Application.configure do # Catch missing translations during test runs. config.i18n.raise_on_missing_translations = true + config.i18n.load_path += Dir[Rails.root.join("spec", "support", "locales", "**", "*.yml")] config.after_initialize do ActiveRecord::LogSubscriber.backtrace_cleaner.add_silencer do |line| diff --git a/config/initializers/002-rails_failover.rb b/config/initializers/002-rails_failover.rb index d5ee5c2da24..d43a24334ac 100644 --- a/config/initializers/002-rails_failover.rb +++ b/config/initializers/002-rails_failover.rb @@ -19,7 +19,7 @@ if defined?(RailsFailover::Redis) SiteSetting.refresh! end - RailsFailover::Redis.logger = Rails.logger.chained.first if Rails.logger.respond_to? :chained + RailsFailover::Redis.logger = Rails.logger.broadcasts.first end if defined?(RailsFailover::ActiveRecord) diff --git a/config/initializers/100-logster.rb b/config/initializers/100-logster.rb index 0a33877d3d5..9248e35e326 100644 --- a/config/initializers/100-logster.rb +++ b/config/initializers/100-logster.rb @@ -2,7 +2,7 @@ if GlobalSetting.skip_redis? Rails.application.reloader.to_prepare do - Rails.logger = Rails.logger.chained.first if Rails.logger.respond_to? :chained + Rails.logger.stop_broadcasting_to(Logster.logger) if defined?(Logster::Logger) && Logster.logger end return end @@ -10,12 +10,12 @@ end if Rails.env.development? && !Sidekiq.server? && ENV["RAILS_LOGS_STDOUT"] == "1" Rails.application.config.after_initialize do console = ActiveSupport::Logger.new(STDOUT) - original_logger = Rails.logger.chained.first + original_logger = Rails.logger.broadcasts.first console.formatter = original_logger.formatter console.level = original_logger.level unless ActiveSupport::Logger.logger_outputs_to?(original_logger, STDOUT) - original_logger.extend(ActiveSupport::Logger.broadcast(console)) + Rails.logger.broadcast_to(console) end end end @@ -118,10 +118,7 @@ RailsMultisite::ConnectionManagement.each_connection do end if Rails.configuration.multisite - if Rails.logger.respond_to? :chained - chained = Rails.logger.chained - chained && chained.first.formatter = RailsMultisite::Formatter.new - end + Rails.logger.broadcasts.first.formatter = RailsMultisite::Formatter.new end Logster.config.project_directories = [ diff --git a/config/initializers/100-quiet_logger.rb b/config/initializers/100-quiet_logger.rb index ef156499861..d72387be9e2 100644 --- a/config/initializers/100-quiet_logger.rb +++ b/config/initializers/100-quiet_logger.rb @@ -11,13 +11,13 @@ module DiscourseRackQuietAssetsLogger (env["PATH_INFO"].index("/service-worker") == 0) || (env["PATH_INFO"].index("mini-profiler-resources") == 0) || (env["PATH_INFO"].index("/srv/status") == 0) - if ::Logster::Logger === Rails.logger + if defined?(::Logster::Logger) && Logster.logger override = true - Rails.logger.override_level = Logger::ERROR + Logster.logger.override_level = Logger::ERROR end end - super(env).tap { Rails.logger.override_level = nil if override } + super(env).tap { Logster.logger.override_level = nil if override } end end diff --git a/config/initializers/100-silence_logger.rb b/config/initializers/100-silence_logger.rb index e01b29816f7..d74386878bd 100644 --- a/config/initializers/100-silence_logger.rb +++ b/config/initializers/100-silence_logger.rb @@ -20,16 +20,21 @@ class SilenceLogger < Rails::Rack::Logger if env[HTTP_X_SILENCE_LOGGER] || @opts[:silenced].include?(path_info) || path_info.start_with?("/logs") || path_info.start_with?("/user_avatar") || path_info.start_with?("/letter_avatar") - if ::Logster::Logger === Rails.logger + if defined?(::Logster::Logger) && Logster.logger override = true - Rails.logger.override_level = Logger::WARN + Logster.logger.override_level = Logger::WARN end @app.call(env) else - super(env) + # TODO: Just call `super` instead when upgrading to Rails 7.2. With Rails + # 7.1 there is a bug in `Rails::Rack::Logger` when there is more than one + # `ActiveSupport::TaggedLogging` logger in the broadcast logger. It + # results in duplicating the response from Rack as many times as there + # are tagged loggers, returning an array of arrays. + super.tap { break _1.first if _1.any?(Array) } end ensure - Rails.logger.override_level = nil if override + Logster.logger.override_level = nil if override end end diff --git a/config/initializers/101-lograge.rb b/config/initializers/101-lograge.rb index 3baac8b4b46..b17cb7f3c77 100644 --- a/config/initializers/101-lograge.rb +++ b/config/initializers/101-lograge.rb @@ -111,8 +111,8 @@ Rails.application.config.to_prepare do # Remove ActiveSupport::Logger from the chain and replace with Lograge's # logger - Rails.logger.chained.pop - Rails.logger.chain(config.lograge.logger) + Rails.logger.stop_broadcasting_to(Rails.logger.broadcasts.last) + Rails.logger.broadcast_to(config.lograge.logger) end end end diff --git a/config/initializers/102-truncate-logs.rb b/config/initializers/102-truncate-logs.rb index b1276f3d967..29601aa1f4b 100644 --- a/config/initializers/102-truncate-logs.rb +++ b/config/initializers/102-truncate-logs.rb @@ -22,10 +22,6 @@ if Rails.env.production? || ENV["ENABLE_LOGS_TRUNCATION"] == "1" end Rails.application.config.to_prepare do - set_or_extend_truncate_logs_formatter(Rails.logger) - - if Rails.logger.respond_to? :chained - Rails.logger.chained.each { |logger| set_or_extend_truncate_logs_formatter(logger) } - end + Rails.logger.broadcasts.each { |logger| set_or_extend_truncate_logs_formatter(logger) } end end diff --git a/config/initializers/new_framework_defaults_7_0.rb b/config/initializers/new_framework_defaults_7_0.rb deleted file mode 100644 index fdbf91e62fc..00000000000 --- a/config/initializers/new_framework_defaults_7_0.rb +++ /dev/null @@ -1,106 +0,0 @@ -# frozen_string_literal: true - -# Be sure to restart your server when you modify this file. -# -# This file eases your Rails 7.0 framework defaults upgrade. -# -# Uncomment each configuration one by one to switch to the new default. -# Once your application is ready to run with all new defaults, you can remove -# this file and set the `config.load_defaults` to `7.0`. -# -# Read the Guide for Upgrading Ruby on Rails for more info on each option. -# https://guides.rubyonrails.org/upgrading_ruby_on_rails.html - -# `button_to` view helper will render `