From df111259fe7edd34e2d72d4484e4cc4f36e6b770 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 14 Nov 2018 20:26:08 +0800 Subject: [PATCH] More URL site settings into a onceoff job. * Doing it in a post migration was a bad idea because the migration will fail if the site is down while trying to download uploads which points to the instance. This mainly affects self-hosters using `discourse_docker` where `./launcher rebuild` will take the existing container down. --- app/jobs/onceoff/migrate_url_site_settings.rb | 76 +++++++++++++++++++ ...0181112013117_migrate_url_site_settings.rb | 75 ------------------ .../migrate_url_site_settings_spec.rb | 12 +-- 3 files changed, 80 insertions(+), 83 deletions(-) create mode 100644 app/jobs/onceoff/migrate_url_site_settings.rb delete mode 100644 db/post_migrate/20181112013117_migrate_url_site_settings.rb rename spec/{db/post_migrate => jobs}/migrate_url_site_settings_spec.rb (87%) diff --git a/app/jobs/onceoff/migrate_url_site_settings.rb b/app/jobs/onceoff/migrate_url_site_settings.rb new file mode 100644 index 00000000000..c40b17983fc --- /dev/null +++ b/app/jobs/onceoff/migrate_url_site_settings.rb @@ -0,0 +1,76 @@ +module Jobs + class MigrateUrlSiteSettings < Jobs::Onceoff + def execute_onceoff(args) + [ + ['logo_url', 'logo'], + ['logo_small_url', 'logo_small'], + ['digest_logo_url', 'digest_logo'], + ['mobile_logo_url', 'mobile_logo'], + ['large_icon_url', 'large_icon'], + ['favicon_url', 'favicon'], + ['apple_touch_icon_url', 'apple_touch_icon'], + ['default_opengraph_image_url', 'opengraph_image'], + ['twitter_summary_large_image_url', 'twitter_summary_large_image'], + ['push_notifications_icon_url', 'push_notifications_icon'], + ].each do |old_setting, new_setting| + old_url = DB.query_single( + "SELECT value FROM site_settings WHERE name = '#{old_setting}'" + ).first + + next if old_url.blank? + + count = 0 + file = nil + sleep_interval = 5 + + loop do + url = UrlHelper.absolute_without_cdn(old_url) + + begin + file = FileHelper.download( + url, + max_file_size: [ + SiteSetting.max_image_size_kb.kilobytes, + 20.megabytes + ].max, + tmp_file_name: 'tmp_site_setting_logo', + skip_rate_limit: true, + follow_redirect: true + ) + rescue OpenURI::HTTPError, Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNREFUSED => e + logger.info( + "Error encountered when trying to download file " + + "for #{new_setting}.\n#{e.class}: #{e.message}\n#{e.backtrace.join("\n")}" + ) + end + + count += 1 + break if file || (file.blank? && count >= 3) + + logger.info( + "Failed to download upload from #{url} for #{new_setting}. Retrying..." + ) + + sleep(count * sleep_interval) + end + + next if file.blank? + + upload = UploadCreator.new( + file, + "#{new_setting}", + origin: UrlHelper.absolute(old_url), + for_site_setting: true + ).create_for(Discourse.system_user.id) + + SiteSetting.public_send("#{new_setting}=", upload) + end + end + + private + + def logger + Rails.logger + end + end +end diff --git a/db/post_migrate/20181112013117_migrate_url_site_settings.rb b/db/post_migrate/20181112013117_migrate_url_site_settings.rb deleted file mode 100644 index fac8fad4fb4..00000000000 --- a/db/post_migrate/20181112013117_migrate_url_site_settings.rb +++ /dev/null @@ -1,75 +0,0 @@ -class MigrateUrlSiteSettings < ActiveRecord::Migration[5.2] - def up - [ - ['logo_url', 'logo'], - ['logo_small_url', 'logo_small'], - ['digest_logo_url', 'digest_logo'], - ['mobile_logo_url', 'mobile_logo'], - ['large_icon_url', 'large_icon'], - ['favicon_url', 'favicon'], - ['apple_touch_icon_url', 'apple_touch_icon'], - ['default_opengraph_image_url', 'opengraph_image'], - ['twitter_summary_large_image_url', 'twitter_summary_large_image'], - ['push_notifications_icon_url', 'push_notifications_icon'], - ].each do |old_setting, new_setting| - old_url = DB.query_single( - "SELECT value FROM site_settings WHERE name = '#{old_setting}'" - ).first - - next if old_url.blank? - - count = 0 - file = nil - sleep_interval = 5 - - loop do - url = UrlHelper.absolute_without_cdn(old_url) - - begin - file = FileHelper.download( - url, - max_file_size: 20.megabytes, - tmp_file_name: 'tmp_site_setting_logo', - skip_rate_limit: true, - follow_redirect: true - ) - rescue OpenURI::HTTPError, Net::OpenTimeout, Net::ReadTimeout => e - logger.info( - "HTTP error encountered when trying to download file " + - "for #{new_setting}.\n#{e.class}: #{e.message}\n#{e.backtrace.join("\n")}" - ) - end - - count += 1 - break if file || (file.blank? && count >= 3) - - logger.info( - "Failed to download upload from #{url} for #{new_setting}. Retrying..." - ) - - sleep(count * sleep_interval) - end - - next if file.blank? - - upload = UploadCreator.new( - file, - "#{new_setting}", - origin: UrlHelper.absolute(old_url), - for_site_setting: true - ).create_for(Discourse.system_user.id) - - SiteSetting.public_send("#{new_setting}=", upload) - end - end - - def down - raise ActiveRecord::IrreversibleMigration - end - - private - - def logger - Rails.logger - end -end diff --git a/spec/db/post_migrate/migrate_url_site_settings_spec.rb b/spec/jobs/migrate_url_site_settings_spec.rb similarity index 87% rename from spec/db/post_migrate/migrate_url_site_settings_spec.rb rename to spec/jobs/migrate_url_site_settings_spec.rb index 8b931c67e0b..823e5b70a92 100644 --- a/spec/db/post_migrate/migrate_url_site_settings_spec.rb +++ b/spec/jobs/migrate_url_site_settings_spec.rb @@ -1,7 +1,6 @@ require 'rails_helper' -require_relative '../../../db/post_migrate/20181112013117_migrate_url_site_settings' -RSpec.describe MigrateUrlSiteSettings do +RSpec.describe Jobs::MigrateUrlSiteSettings do before do SiteSetting.authorized_extensions = '' end @@ -42,12 +41,9 @@ RSpec.describe MigrateUrlSiteSettings do stub_request(:get, "http://test.discourse.awesome/some.ico") .to_return(status: 200, body: file_from_fixtures("smallest.ico").read) - begin - STDOUT.stubs(:write) - expect { MigrateUrlSiteSettings.new.up }.to change { Upload.count }.by(3) - ensure - STDOUT.unstub(:write) - end + expect do + described_class.new.execute_onceoff({}) + end.to change { Upload.count }.by(3) upload = Upload.find_by(original_filename: "logo.png") upload2 = Upload.find_by(original_filename: "logo_small.png")