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

@@ -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

db/post_migrate/20181112013117_migrate_url_site_settings.rb

@@ -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

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
+    expect do
-      STDOUT.stubs(:write)
+      described_class.new.execute_onceoff({})
-      expect { MigrateUrlSiteSettings.new.up }.to change { Upload.count }.by(3)
+    end.to change { Upload.count }.by(3)
-    ensure
-      STDOUT.unstub(:write)
-    end
 
     upload = Upload.find_by(original_filename: "logo.png")
     upload2 = Upload.find_by(original_filename: "logo_small.png")