From 32b8a2ccffd50d78b8f7be6f0b524506cd2dbd7e Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Thu, 12 Sep 2019 10:41:50 +1000 Subject: [PATCH] DEV: Upgrade Discourse to Rails 6 (#8083) * Adjustments to pass specs on Rails 6.0.0 * Use classic autoloader instead of Zeitwerk * Update Rails 6.0.0 deprecated methods * Rails 6.0.0 not allowing column with integer name * Drop freedom_patches/rails6.rb * Default value for trigger_transactional_callbacks? is true * Bump rspec-rails version to 4.0.0.beta2 --- Gemfile | 16 +-- Gemfile.lock | 88 +++++++------- app/views/users/show.html.erb | 2 +- config/application.rb | 2 + ...20130430052751_add_topic_allowed_groups.rb | 4 +- ...20130501105651_fix_topic_allowed_groups.rb | 2 +- lib/freedom_patches/rails6.rb | 110 ------------------ lib/mini_sql_multisite_connection.rb | 3 + .../components/migration/safe_migrate_spec.rb | 4 +- spec/components/post_revisor_spec.rb | 2 +- spec/mailers/user_notifications_spec.rb | 2 +- spec/requests/badges_controller_spec.rb | 2 +- spec/requests/groups_controller_spec.rb | 4 +- spec/requests/list_controller_spec.rb | 8 +- spec/requests/metadata_controller_spec.rb | 8 +- spec/requests/static_controller_spec.rb | 6 +- spec/requests/topics_controller_spec.rb | 2 +- 17 files changed, 80 insertions(+), 185 deletions(-) delete mode 100644 lib/freedom_patches/rails6.rb diff --git a/Gemfile b/Gemfile index 1e9fe98481..98e165f3ac 100644 --- a/Gemfile +++ b/Gemfile @@ -16,13 +16,13 @@ if rails_master? else # until rubygems gives us optional dependencies we are stuck with this # bundle update actionmailer actionpack actionview activemodel activerecord activesupport railties - gem 'actionmailer', '5.2.3' - gem 'actionpack', '5.2.3' - gem 'actionview', '5.2.3' - gem 'activemodel', '5.2.3' - gem 'activerecord', '5.2.3' - gem 'activesupport', '5.2.3' - gem 'railties', '5.2.3' + gem 'actionmailer', '6.0.0' + gem 'actionpack', '6.0.0' + gem 'actionview', '6.0.0' + gem 'activemodel', '6.0.0' + gem 'activerecord', '6.0.0' + gem 'activesupport', '6.0.0' + gem 'railties', '6.0.0' gem 'sprockets-rails' end @@ -140,7 +140,7 @@ group :test, :development do gem 'mocha', require: false gem 'rb-fsevent', require: RUBY_PLATFORM =~ /darwin/i ? 'rb-fsevent' : false gem 'rb-inotify', '~> 0.9', require: RUBY_PLATFORM =~ /linux/i ? 'rb-inotify' : false - gem 'rspec-rails', require: false + gem 'rspec-rails', '4.0.0.beta2', require: false gem 'shoulda-matchers', '~> 3.1', '>= 3.1.3', require: false gem 'rspec-html-matchers' gem 'pry-nav' diff --git a/Gemfile.lock b/Gemfile.lock index 7fc71a5425..55e2d279a3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,47 +1,46 @@ GEM remote: https://rubygems.org/ specs: - actionmailer (5.2.3) - actionpack (= 5.2.3) - actionview (= 5.2.3) - activejob (= 5.2.3) + actionmailer (6.0.0) + actionpack (= 6.0.0) + actionview (= 6.0.0) + activejob (= 6.0.0) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (5.2.3) - actionview (= 5.2.3) - activesupport (= 5.2.3) + actionpack (6.0.0) + actionview (= 6.0.0) + activesupport (= 6.0.0) rack (~> 2.0) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (5.2.3) - activesupport (= 5.2.3) + rails-html-sanitizer (~> 1.0, >= 1.2.0) + actionview (6.0.0) + activesupport (= 6.0.0) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.0, >= 1.0.3) + rails-html-sanitizer (~> 1.1, >= 1.2.0) active_model_serializers (0.8.4) activemodel (>= 3.0) - activejob (5.2.3) - activesupport (= 5.2.3) + activejob (6.0.0) + activesupport (= 6.0.0) globalid (>= 0.3.6) - activemodel (5.2.3) - activesupport (= 5.2.3) - activerecord (5.2.3) - activemodel (= 5.2.3) - activesupport (= 5.2.3) - arel (>= 9.0) - activesupport (5.2.3) + activemodel (6.0.0) + activesupport (= 6.0.0) + activerecord (6.0.0) + activemodel (= 6.0.0) + activesupport (= 6.0.0) + activesupport (6.0.0) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) tzinfo (~> 1.1) + zeitwerk (~> 2.1, >= 2.1.8) addressable (2.5.2) public_suffix (>= 2.0.2, < 4.0) annotate (2.7.5) activerecord (>= 3.2, < 7.0) rake (>= 10.4, < 13.0) - arel (9.0.0) ast (2.4.0) aws-eventstream (1.0.3) aws-partitions (1.154.0) @@ -183,7 +182,7 @@ GEM rack (>= 1.1.3) metaclass (0.0.4) method_source (0.9.2) - mini_mime (1.0.1) + mini_mime (1.0.2) mini_portile2 (2.4.0) mini_racer (0.2.6) libv8 (>= 6.9.411) @@ -282,20 +281,20 @@ GEM rails-dom-testing (2.0.3) activesupport (>= 4.2.0) nokogiri (>= 1.6) - rails-html-sanitizer (1.0.4) + rails-html-sanitizer (1.2.0) loofah (~> 2.2, >= 2.2.2) rails_multisite (2.0.7) activerecord (> 4.2, < 7) railties (> 4.2, < 7) - railties (5.2.3) - actionpack (= 5.2.3) - activesupport (= 5.2.3) + railties (6.0.0) + actionpack (= 6.0.0) + activesupport (= 6.0.0) method_source rake (>= 0.8.7) - thor (>= 0.19.0, < 2.0) + thor (>= 0.20.3, < 2.0) rainbow (3.0.0) raindrops (0.19.0) - rake (12.3.2) + rake (12.3.3) rake-compiler (1.0.7) rake rb-fsevent (0.10.3) @@ -330,14 +329,14 @@ GEM rspec-mocks (3.8.0) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.8.0) - rspec-rails (3.8.2) - actionpack (>= 3.0) - activesupport (>= 3.0) - railties (>= 3.0) - rspec-core (~> 3.8.0) - rspec-expectations (~> 3.8.0) - rspec-mocks (~> 3.8.0) - rspec-support (~> 3.8.0) + rspec-rails (4.0.0.beta2) + actionpack (>= 4.2) + activesupport (>= 4.2) + railties (>= 4.2) + rspec-core (~> 3.8) + rspec-expectations (~> 3.8) + rspec-mocks (~> 3.8) + rspec-support (~> 3.8) rspec-support (3.8.0) rtlit (0.0.5) rubocop (0.69.0) @@ -417,18 +416,19 @@ GEM hkdf (~> 0.2) jwt (~> 2.0) yaml-lint (0.0.10) + zeitwerk (2.1.10) PLATFORMS ruby DEPENDENCIES - actionmailer (= 5.2.3) - actionpack (= 5.2.3) - actionview (= 5.2.3) + actionmailer (= 6.0.0) + actionpack (= 6.0.0) + actionview (= 6.0.0) active_model_serializers (~> 0.8.3) - activemodel (= 5.2.3) - activerecord (= 5.2.3) - activesupport (= 5.2.3) + activemodel (= 6.0.0) + activerecord (= 6.0.0) + activesupport (= 6.0.0) annotate aws-sdk-s3 aws-sdk-sns @@ -504,7 +504,7 @@ DEPENDENCIES rack-mini-profiler rack-protection rails_multisite - railties (= 5.2.3) + railties (= 6.0.0) rake rb-fsevent rb-inotify (~> 0.9) @@ -517,7 +517,7 @@ DEPENDENCIES rqrcode rspec rspec-html-matchers - rspec-rails + rspec-rails (= 4.0.0.beta2) rtlit rubocop ruby-prof diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 55f9b38a17..007a600d54 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -1,5 +1,5 @@
- <%= @user.username %> + <%= @user.username %>

<%= @user.username %>

diff --git a/config/application.rb b/config/application.rb index 2d946b30ec..932aed2806 100644 --- a/config/application.rb +++ b/config/application.rb @@ -93,6 +93,8 @@ module Discourse # issue is image_optim crashes on missing dependencies config.assets.image_optim = false + config.autoloader = :classic + # Custom directories with classes and modules you want to be autoloadable. config.autoload_paths += Dir["#{config.root}/app/serializers"] config.autoload_paths += Dir["#{config.root}/lib/validators/"] diff --git a/db/migrate/20130430052751_add_topic_allowed_groups.rb b/db/migrate/20130430052751_add_topic_allowed_groups.rb index 27eb25054e..aae17685c6 100644 --- a/db/migrate/20130430052751_add_topic_allowed_groups.rb +++ b/db/migrate/20130430052751_add_topic_allowed_groups.rb @@ -4,8 +4,8 @@ class AddTopicAllowedGroups < ActiveRecord::Migration[4.2] def change create_table :topic_allowed_groups, force: true do |t| # oops - t.integer :group_id, :integer, null: false - t.integer :topic_id, :integer, null: false + t.integer :group_id, null: false + t.integer :topic_id, null: false end add_index :topic_allowed_groups, [:group_id, :topic_id], unique: true diff --git a/db/migrate/20130501105651_fix_topic_allowed_groups.rb b/db/migrate/20130501105651_fix_topic_allowed_groups.rb index cc0f49fbf0..bb19a27651 100644 --- a/db/migrate/20130501105651_fix_topic_allowed_groups.rb +++ b/db/migrate/20130501105651_fix_topic_allowed_groups.rb @@ -3,6 +3,6 @@ class FixTopicAllowedGroups < ActiveRecord::Migration[4.2] def change # big oops - remove_column :topic_allowed_groups, :integer + remove_column :topic_allowed_groups, :integer if column_exists?(:topic_allowed_groups, :integer) end end diff --git a/lib/freedom_patches/rails6.rb b/lib/freedom_patches/rails6.rb deleted file mode 100644 index f8c188ea93..0000000000 --- a/lib/freedom_patches/rails6.rb +++ /dev/null @@ -1,110 +0,0 @@ -# frozen_string_literal: true - -# this is a quick backport of a new method introduced in Rails 6 -# to be removed after we upgrade to Rails 6 -if ! defined? ActionView::Base.with_view_paths - class ActionView::Base - class << self - alias with_view_paths new - end - end -end - -# backport of https://github.com/rails/rails/commit/890485cfce4c361c03a41ec23b0ba187007818cc -if !defined? ActionDispatch::Http::ContentDisposition - module ActionDispatch - module Http - class ContentDisposition - def self.format(disposition:, filename:) - new(disposition: disposition, filename: filename).to_s - end - - attr_reader :disposition, :filename - - def initialize(disposition:, filename:) - @disposition = disposition - @filename = filename - end - - TRADITIONAL_ESCAPED_CHAR = /[^ A-Za-z0-9!#$+.^_`|~-]/ - - def ascii_filename - 'filename="' + percent_escape(I18n.transliterate(filename), TRADITIONAL_ESCAPED_CHAR) + '"' - end - - RFC_5987_ESCAPED_CHAR = /[^A-Za-z0-9!#$&+.^_`|~-]/ - - def utf8_filename - "filename*=UTF-8''" + percent_escape(filename, RFC_5987_ESCAPED_CHAR) - end - - def to_s - if filename - "#{disposition}; #{ascii_filename}; #{utf8_filename}" - else - "#{disposition}" - end - end - - private - def percent_escape(string, pattern) - string.gsub(pattern) do |char| - char.bytes.map { |byte| "%%%02X" % byte }.join - end - end - end - end - end - - module ActionController - module DataStreaming - private - def send_file_headers!(options) - type_provided = options.has_key?(:type) - - content_type = options.fetch(:type, DEFAULT_SEND_FILE_TYPE) - self.content_type = content_type - response.sending_file = true - - raise ArgumentError, ":type option required" if content_type.nil? - - if content_type.is_a?(Symbol) - extension = Mime[content_type] - raise ArgumentError, "Unknown MIME type #{options[:type]}" unless extension - self.content_type = extension - else - if !type_provided && options[:filename] - # If type wasn't provided, try guessing from file extension. - content_type = Mime::Type.lookup_by_extension(File.extname(options[:filename]).downcase.delete(".")) || content_type - end - self.content_type = content_type - end - - disposition = options.fetch(:disposition, DEFAULT_SEND_FILE_DISPOSITION) - if disposition - headers["Content-Disposition"] = ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: options[:filename]) - end - - headers["Content-Transfer-Encoding"] = "binary" - - # Fix a problem with IE 6.0 on opening downloaded files: - # If Cache-Control: no-cache is set (which Rails does by default), - # IE removes the file it just downloaded from its cache immediately - # after it displays the "open/save" dialog, which means that if you - # hit "open" the file isn't there anymore when the application that - # is called for handling the download is run, so let's workaround that - response.cache_control[:public] ||= false - end - end - end - - module ActiveStorage - class Service - private - def content_disposition_with(type: "inline", filename:) - disposition = (type.to_s.presence_in(%w( attachment inline )) || "inline") - ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: filename.sanitized) - end - end - end -end diff --git a/lib/mini_sql_multisite_connection.rb b/lib/mini_sql_multisite_connection.rb index ee1129c65f..c08d3813a9 100644 --- a/lib/mini_sql_multisite_connection.rb +++ b/lib/mini_sql_multisite_connection.rb @@ -36,6 +36,9 @@ class MiniSqlMultisiteConnection < MiniSql::Postgres::Connection def before_committed!(*); end def rolledback!(*); end + def trigger_transactional_callbacks? + true + end end # Allows running arbitrary code after the current transaction has been committed. diff --git a/spec/components/migration/safe_migrate_spec.rb b/spec/components/migration/safe_migrate_spec.rb index 5c04f5cec5..9bd0b690ae 100644 --- a/spec/components/migration/safe_migrate_spec.rb +++ b/spec/components/migration/safe_migrate_spec.rb @@ -14,8 +14,8 @@ describe Migration::SafeMigrate do end def migrate_up(path) - migrations = ActiveRecord::MigrationContext.new(path).migrations - ActiveRecord::Migrator.new(:up, migrations, migrations.first.version).run + migrations = ActiveRecord::MigrationContext.new(path, ActiveRecord::SchemaMigration).migrations + ActiveRecord::Migrator.new(:up, migrations, ActiveRecord::SchemaMigration, migrations.first.version).run end it "bans all table removal" do diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index ed4f2887e7..0f15c4b720 100644 --- a/spec/components/post_revisor_spec.rb +++ b/spec/components/post_revisor_spec.rb @@ -734,7 +734,7 @@ describe PostRevisor do let(:bumped_at) { 1.day.ago } before do - topic.update_attributes!(bumped_at: bumped_at) + topic.update!(bumped_at: bumped_at) create_hidden_tags(['important', 'secret']) topic = post.topic topic.tags = [Fabricate(:tag, name: "super"), Tag.where(name: "important").first, Fabricate(:tag, name: "stuff")] diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index 420655b2ac..f0640a0335 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -344,7 +344,7 @@ describe UserNotifications do ] SiteSetting.post_excerpts_in_emails = true SiteSetting.post_excerpt_maxlength = paragraphs.first.length - response.update_attributes!(raw: paragraphs.join("\n\n")) + response.update!(raw: paragraphs.join("\n\n")) mail = UserNotifications.user_replied( user, post: response, diff --git a/spec/requests/badges_controller_spec.rb b/spec/requests/badges_controller_spec.rb index 8d04b98d80..16abe66686 100644 --- a/spec/requests/badges_controller_spec.rb +++ b/spec/requests/badges_controller_spec.rb @@ -39,7 +39,7 @@ describe BadgesController do it 'renders rss feed of a badge' do get "/badges/#{badge.id}.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end end end diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index 124eaf5c7d..42d76b4d50 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -426,7 +426,7 @@ describe GroupsController do get "/groups/#{group.name}/posts.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end end @@ -435,7 +435,7 @@ describe GroupsController do get "/groups/#{group.name}/mentions.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end it 'fails when disabled' do diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index dd94355045..9727a06d4c 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -320,7 +320,7 @@ RSpec.describe ListController do it 'renders latest RSS' do get "/latest.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end it 'renders links correctly with subfolder' do @@ -337,14 +337,14 @@ RSpec.describe ListController do it 'renders top RSS' do get "/top.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end TopTopic.periods.each do |period| it "renders #{period} top RSS" do get "/top/#{period}.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end end end @@ -433,7 +433,7 @@ RSpec.describe ListController do it 'renders RSS' do get "/c/#{category.slug}.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end it "renders RSS in subfolder correctly" do diff --git a/spec/requests/metadata_controller_spec.rb b/spec/requests/metadata_controller_spec.rb index eaa8f05da2..041be7ebe8 100644 --- a/spec/requests/metadata_controller_spec.rb +++ b/spec/requests/metadata_controller_spec.rb @@ -20,7 +20,7 @@ RSpec.describe MetadataController do get "/manifest.webmanifest" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/manifest+json') + expect(response.media_type).to eq('application/manifest+json') manifest = JSON.parse(response.body) expect(manifest["name"]).to eq(title) @@ -98,7 +98,7 @@ RSpec.describe MetadataController do expect(response.body).to include("/search?q={searchTerms}") expect(response.body).to include('image/png') expect(response.body).to include(UrlHelper.absolute(upload.url)) - expect(response.content_type).to eq('application/xml') + expect(response.media_type).to eq('application/xml') end end @@ -121,7 +121,7 @@ RSpec.describe MetadataController do expect(response.status).to eq(200) expect(response.body).to include("hash_of_app_certificate") expect(response.body).to include("com.example.app") - expect(response.content_type).to eq('application/json') + expect(response.media_type).to eq('application/json') end end @@ -143,7 +143,7 @@ RSpec.describe MetadataController do expect(response.status).to eq(200) expect(response.body).to include("applinks") - expect(response.content_type).to eq('application/json') + expect(response.media_type).to eq('application/json') get "/apple-app-site-association.json" expect(response.status).to eq(404) diff --git a/spec/requests/static_controller_spec.rb b/spec/requests/static_controller_spec.rb index 6fcb784ab7..dff4e02959 100644 --- a/spec/requests/static_controller_spec.rb +++ b/spec/requests/static_controller_spec.rb @@ -26,7 +26,7 @@ describe StaticController do get '/favicon/proxied' expect(response.status).to eq(200) - expect(response.content_type).to eq('image/png') + expect(response.media_type).to eq('image/png') expect(response.body.bytesize).to eq(SiteIconManager.favicon.filesize) end @@ -36,7 +36,7 @@ describe StaticController do get '/favicon/proxied' expect(response.status).to eq(200) - expect(response.content_type).to eq('image/png') + expect(response.media_type).to eq('image/png') expect(response.body.bytesize).to eq(upload.filesize) end end @@ -66,7 +66,7 @@ describe StaticController do get '/favicon/proxied' expect(response.status).to eq(200) - expect(response.content_type).to eq('image/png') + expect(response.media_type).to eq('image/png') expect(response.body.bytesize).to eq(upload.filesize) end end diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 5ea6b9b6f0..804952ef25 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -1998,7 +1998,7 @@ RSpec.describe TopicsController do it 'renders rss of the topic' do get "/t/foo/#{topic.id}.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end it 'renders rss of the topic correctly with subfolder' do