From 81571bb0b5c6e8e429215f0311dbc2b8ecfdcf40 Mon Sep 17 00:00:00 2001 From: Ridhwana Date: Mon, 10 Jan 2022 16:09:34 +0200 Subject: [PATCH] Data Update Script to migrate logo_svg contents to png file (Will be merged and deployed on 10/01) (#15710) * feat/WIP: first version of the svg logo to png logo DUS * feat: logoSVG Uploader * refactor: remove the if original_filename * feat: add a test for the logo_svg_uploader * feat: update the test and dus * add soem error handling * updae the rails spec helper * feat: provide a content type * feat: add content type * Try https://travis-ci.community/t/build-times-out-with-no-apparent-reason/5083/4 * chore: try this suggestion https://stackoverflow.com/questions/41138404/how-to-install-newer-imagemagick-with-webp-support-in-travis-ci-container * chore: remove libweb adn fix = in the travis.yml * chore: undo the changes to attempt to update the version for image magick * feat: install gsfonts for convert * chore: add a comment for svg * feat: use the good practices for tempfile, update error handling and update the tests * chore: update the timestamp on the DUS * feat: convert to png with a transparent background using Image Magick * feat: substitute some css variables with real css colors * chore: revert the path * fix: set content type * Update lib/data_update_scripts/20220105112823_migrate_logo_svg_data.rb Co-authored-by: Jamie Gaskins Co-authored-by: Jamie Gaskins --- .travis.yml | 2 + app/uploaders/base_uploader.rb | 3 + app/uploaders/logo_svg_uploader.rb | 65 +++++++++++++++++++ app/uploaders/logo_uploader.rb | 4 +- .../20220105112823_migrate_logo_svg_data.rb | 38 +++++++++++ .../migrate_logo_svg_data_spec.rb | 42 ++++++++++++ spec/uploaders/logo_svg_uploader_spec.rb | 56 ++++++++++++++++ spec/uploaders/logo_uploader_spec.rb | 2 +- 8 files changed, 209 insertions(+), 3 deletions(-) create mode 100644 app/uploaders/logo_svg_uploader.rb create mode 100644 lib/data_update_scripts/20220105112823_migrate_logo_svg_data.rb create mode 100644 spec/lib/data_update_scripts/migrate_logo_svg_data_spec.rb create mode 100644 spec/uploaders/logo_svg_uploader_spec.rb diff --git a/.travis.yml b/.travis.yml index ef80efb34..36959458d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -44,6 +44,8 @@ env: - FOREM_OWNER_SECRET="secret" # test secret so e2e tests can run properly. - DISABLE_SPRING=true before_install: + - sudo apt-get update + - sudo apt-get -y install gsfonts - >- sudo sed -i -e '/local.*peer/s/postgres/all/' diff --git a/app/uploaders/base_uploader.rb b/app/uploaders/base_uploader.rb index 52e2e7674..bd5a22661 100644 --- a/app/uploaders/base_uploader.rb +++ b/app/uploaders/base_uploader.rb @@ -26,6 +26,9 @@ class BaseUploader < CarrierWave::Uploader::Base # strip EXIF (and GPS) data def strip_exif + # There will be no exif data for an SVG + return if file.content_type.include?("svg") + manipulate! do |image| image.strip unless image.frames.count > FRAME_STRIP_MAX image = yield(image) if block_given? diff --git a/app/uploaders/logo_svg_uploader.rb b/app/uploaders/logo_svg_uploader.rb new file mode 100644 index 000000000..e119f749e --- /dev/null +++ b/app/uploaders/logo_svg_uploader.rb @@ -0,0 +1,65 @@ +class LogoSvgUploader < BaseUploader + # This Uploader is being used by a Data Update Script + # lib/data_update_scripts/20211208074428_migrate_logo_svg_data.rb + # to convert logo_svg contents to a png file. + + STORE_DIRECTORY = "uploads/logos/".freeze + EXTENSION_ALLOWLIST = %w[svg png].freeze + IMAGE_TYPE_ALLOWLIST = %i[svg png].freeze + CONTENT_TYPE_ALLOWLIST = %w[image/svg+xml image/png].freeze + + process :convert_to_png + + def store_dir + STORE_DIRECTORY + end + + def extension_allowlist + EXTENSION_ALLOWLIST + end + + def image_type_whitelist + # this is needed by CarrierWave::BombShelter + IMAGE_TYPE_ALLOWLIST + end + + def content_type_allowlist + CONTENT_TYPE_ALLOWLIST + end + + def filename + # random_string in the filename to avoid caching issues + "original_logo_#{random_string}.png" + end + + def convert_to_png + temp_png_file = Tempfile.new ["output", ".png"] + + begin + MiniMagick::Tool::Convert.new do |convert| + convert.background("none") + convert << file.path + convert << temp_png_file.path + end + + # https://github.com/minimagick/minimagick/issues/332 + file.instance_variable_set(:@file, temp_png_file) + file.instance_variable_set(:@content_type, "image/png") + ensure + temp_png_file.close + end + end + + version :resized_logo do + process resize_to_limit: [nil, 80] + def full_filename(_for_file = file) + "resized_logo_#{random_string}.png" + end + end + + private + + def random_string + SecureRandom.alphanumeric(20) + end +end diff --git a/app/uploaders/logo_uploader.rb b/app/uploaders/logo_uploader.rb index fd8e12410..5a8303c74 100644 --- a/app/uploaders/logo_uploader.rb +++ b/app/uploaders/logo_uploader.rb @@ -29,13 +29,13 @@ class LogoUploader < BaseUploader def filename # random_string in the filename to avoid caching issues - "original_logo_#{random_string}.#{file.extension}" if original_filename + "original_logo_#{random_string}.#{file.extension}" end version :resized_logo do process resize_to_limit: [nil, 80] def full_filename(_for_file = file) - "resized_logo_#{random_string}.#{file.extension}" if original_filename + "resized_logo_#{random_string}.#{file.extension}" end end diff --git a/lib/data_update_scripts/20220105112823_migrate_logo_svg_data.rb b/lib/data_update_scripts/20220105112823_migrate_logo_svg_data.rb new file mode 100644 index 000000000..008542d99 --- /dev/null +++ b/lib/data_update_scripts/20220105112823_migrate_logo_svg_data.rb @@ -0,0 +1,38 @@ +module DataUpdateScripts + class MigrateLogoSvgData + def run + return if ::Settings::General.try(:logo_svg).blank? || + (::Settings::General.try(:original_logo).present? && ::Settings::General.try(:resized_logo).present?) + + logo_svg = Settings::General.logo_svg + + # We do our best effort here: These are some (possibly not all) of the css variables that we may see in Forem + # logos. Hence, we try and replace them before converting. + logo_svg.gsub! "currentColor", "#090909" + logo_svg.gsub! "var(--base-inverted)", "#fff" + logo_svg.gsub! "var(--link-color)", "#3d3d3d" + + Tempfile.create(["logo", ".svg"]) do |file| + file.write(logo_svg) + + logo_svg_uploader = LogoSvgUploader.new.tap do |uploader| + uploader.store!(file) + end + + ::Settings::General.original_logo = logo_svg_uploader.url + ::Settings::General.resized_logo = logo_svg_uploader.resized_logo.url + end + rescue StandardError => e + # If we can't convert the logo, then alert ourselves so that we can track the Forems + # whose logos could not be converted. + + Rails.logger.error("Could not convert logo_svg for #{Settings::Community.community_name} Forem to a PNG.") + + Honeybadger.notify(e, context: { + community_name: Settings::Community.community_name, + app_domain: Settings::General.app_domain, + }, + tags: "failed_svg_conversion") + end + end +end diff --git a/spec/lib/data_update_scripts/migrate_logo_svg_data_spec.rb b/spec/lib/data_update_scripts/migrate_logo_svg_data_spec.rb new file mode 100644 index 000000000..b6ae63068 --- /dev/null +++ b/spec/lib/data_update_scripts/migrate_logo_svg_data_spec.rb @@ -0,0 +1,42 @@ +require "rails_helper" +require Rails.root.join( + "lib/data_update_scripts/20220105112823_migrate_logo_svg_data.rb", +) + +describe DataUpdateScripts::MigrateLogoSvgData do + # This simply tests the output of the script, however, + # the manner in which the uploader behaves is tested by logo_svg_uploader_spec.rb + + context "when the svg is able to be processed" do + processable_svg_string = ' + + + ' + + it "returns the logo url with a png extension", :aggregate_failures do + allow(Settings::General).to receive(:logo_svg).and_return(processable_svg_string) + described_class.new.run + + expect(::Settings::General.original_logo).to include(".png") + expect(::Settings::General.resized_logo).to include(".png") + end + end + + context "when the svg is unable to be processed" do + # rubocop:disable Layout/LineLength + unprocessable_svg_string = '' + # rubocop:enable Layout/LineLength + + it "logs an error" do + allow(Settings::General).to receive(:logo_svg).and_return(unprocessable_svg_string) + allow(Honeybadger).to receive(:notify) + allow(LogoSvgUploader).to receive(:new).and_raise(StandardError) + + described_class.new.run + expect(Honeybadger).to have_received(:notify).once + expect(::Settings::General.original_logo).to be(nil) + expect(::Settings::General.resized_logo).to be(nil) + end + end +end diff --git a/spec/uploaders/logo_svg_uploader_spec.rb b/spec/uploaders/logo_svg_uploader_spec.rb new file mode 100644 index 000000000..83cb8fcc0 --- /dev/null +++ b/spec/uploaders/logo_svg_uploader_spec.rb @@ -0,0 +1,56 @@ +require "rails_helper" +require "carrierwave/test/matchers" +require "exifr/jpeg" + +describe LogoSvgUploader, type: :uploader do + include CarrierWave::Test::Matchers + + let(:image_svg) { fixture_file_upload("300x100.svg", "image/svg+xml") } + let(:image_webp) { fixture_file_upload("800x600.webp", "image/webp") } + + # we need a new uploader before each test, and since the uploader is not a model + # we can recreate it quickly in memory with `let!` + let!(:uploader) { described_class.new } + + before do + described_class.include CarrierWave::MiniMagick # needed for processing + described_class.enable_processing = true + end + + after do + described_class.enable_processing = false + uploader.remove! + end + + it "stores files in the correct directory" do + expect(uploader.store_dir).to eq("uploads/logos/") + end + + describe "error handling" do + it "raises a CarrierWave error which can be parsed if MiniMagick timeout occurs" do + allow(MiniMagick::Image).to receive(:new).and_raise(Timeout::Error) + + expect { uploader.store!(image_svg) }.to raise_error(CarrierWave::IntegrityError, /Image processing timed out/) + end + end + + describe "processed images" do + it "creates versions of the image with different filenames", :aggregate_failures do + uploader.store!(image_svg) + expect(uploader.filename).to match(/original_logo/) + expect(uploader.resized_logo.file.filename).to match(/resized_logo/) + end + + it "stores the processed logo's with a png file extension" do + uploader.store!(image_svg) + expect(uploader.filename).to match(/\.png\z/) + expect(uploader.resized_logo.file.filename).to match(/\.png\z/) + end + + it "stores the processed logo as a png content type" do + uploader.store!(image_svg) + expect(uploader.content_type).to match(/png/) + expect(uploader.resized_logo.file.content_type).to match(/png/) + end + end +end diff --git a/spec/uploaders/logo_uploader_spec.rb b/spec/uploaders/logo_uploader_spec.rb index 73ba1b99f..941bdfe79 100644 --- a/spec/uploaders/logo_uploader_spec.rb +++ b/spec/uploaders/logo_uploader_spec.rb @@ -5,7 +5,7 @@ require "exifr/jpeg" describe LogoUploader, type: :uploader do include CarrierWave::Test::Matchers - let(:image_svg) { fixture_file_upload("300x100.svg") } + let(:image_svg) { fixture_file_upload("300x100.svg", "image/svg+xml") } let(:image_jpg) { fixture_file_upload("800x600.jpg", "image/jpeg") } let(:image_png) { fixture_file_upload("800x600.png", "image/png") } let(:image_webp) { fixture_file_upload("800x600.webp", "image/webp") }