From 9641d2413d2b221f943c9f8537e36bd3db0241ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Thu, 11 May 2017 00:16:57 +0200 Subject: [PATCH] REFACTOR: upload workflow creation into UploadCreator - Automatically convert large-ish PNG/BMP to JPEG - Updated fast_image to latest version --- Gemfile | 3 +- Gemfile.lock | 4 +- app/controllers/admin/emojis_controller.rb | 10 +- app/controllers/admin/themes_controller.rb | 16 +- app/controllers/uploads_controller.rb | 128 +++------- app/jobs/onceoff/migrate_custom_emojis.rb | 10 +- app/jobs/regular/create_avatar_thumbnails.rb | 20 ++ app/jobs/regular/create_thumbnails.rb | 37 --- app/jobs/regular/pull_hotlinked_images.rb | 3 +- app/models/optimized_image.rb | 2 +- app/models/remote_theme.rb | 3 +- app/models/upload.rb | 196 +------------- app/models/user_avatar.rb | 5 +- config/locales/server.en.yml | 6 +- config/site_settings.yml | 5 +- lib/email/receiver.rb | 3 +- lib/tasks/uploads.rake | 4 +- lib/upload_creator.rb | 255 +++++++++++++++++++ script/import_scripts/base/uploader.rb | 2 +- script/import_scripts/lithium.rb | 6 +- script/import_scripts/mbox.rb | 4 +- script/import_scripts/sfn.rb | 2 +- script/import_scripts/vbulletin.rb | 4 +- script/import_scripts/vbulletin5.rb | 46 ++-- spec/controllers/uploads_controller_spec.rb | 13 +- spec/models/theme_spec.rb | 2 +- spec/models/upload_spec.rb | 85 ------- 27 files changed, 391 insertions(+), 483 deletions(-) create mode 100644 app/jobs/regular/create_avatar_thumbnails.rb delete mode 100644 app/jobs/regular/create_thumbnails.rb create mode 100644 lib/upload_creator.rb diff --git a/Gemfile b/Gemfile index a2aef51033f..8e1ae6db5f9 100644 --- a/Gemfile +++ b/Gemfile @@ -61,8 +61,7 @@ gem 'fast_xs' gem 'fast_xor' -# while we sort out https://github.com/sdsykes/fastimage/pull/46 -gem 'discourse_fastimage', '2.0.3', require: 'fastimage' +gem 'fastimage', '2.1.0' gem 'aws-sdk', require: false gem 'excon', require: false gem 'unf', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 41ada79517a..5e75fca2a7e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -78,7 +78,6 @@ GEM diff-lcs (1.3) discourse-qunit-rails (0.0.9) railties - discourse_fastimage (2.0.3) discourse_image_optim (0.24.4) exifr (~> 1.2, >= 1.2.2) fspath (~> 3.0) @@ -114,6 +113,7 @@ GEM rake rake-compiler fast_xs (0.8.0) + fastimage (2.1.0) ffi (1.9.18) flamegraph (0.9.5) foreman (0.82.0) @@ -402,7 +402,6 @@ DEPENDENCIES byebug certified discourse-qunit-rails - discourse_fastimage (= 2.0.3) discourse_image_optim email_reply_trimmer (= 0.1.6) ember-handlebars-template (= 0.7.5) @@ -415,6 +414,7 @@ DEPENDENCIES fast_blank fast_xor fast_xs + fastimage (= 2.1.0) flamegraph foreman gc_tracer diff --git a/app/controllers/admin/emojis_controller.rb b/app/controllers/admin/emojis_controller.rb index 2674936f6e3..8bd3f36edec 100644 --- a/app/controllers/admin/emojis_controller.rb +++ b/app/controllers/admin/emojis_controller.rb @@ -1,3 +1,5 @@ +require_dependency 'upload_creator' + class Admin::EmojisController < Admin::AdminController def index @@ -14,13 +16,11 @@ class Admin::EmojisController < Admin::AdminController .gsub(/_{2,}/, '_') .downcase - upload = Upload.create_for( - current_user.id, + upload = UploadCreator.new( file.tempfile, file.original_filename, - File.size(file.tempfile.path), - image_type: 'custom_emoji' - ) + type: 'custom_emoji' + ).create_for(current_user.id) data = if upload.persisted? diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb index e4de8709bbd..493bb708ec7 100644 --- a/app/controllers/admin/themes_controller.rb +++ b/app/controllers/admin/themes_controller.rb @@ -1,3 +1,5 @@ +require_dependency 'upload_creator' + class Admin::ThemesController < Admin::AdminController skip_before_filter :check_xhr, only: [:show, :preview] @@ -5,27 +7,23 @@ class Admin::ThemesController < Admin::AdminController def preview @theme = Theme.find(params[:id]) - redirect_to path("/"), flash: {preview_theme_key: @theme.key} + redirect_to path("/"), flash: { preview_theme_key: @theme.key } end def upload_asset path = params[:file].path File.open(path) do |file| - upload = Upload.create_for(current_user.id, - file, - params[:file]&.original_filename || File.basename(path), - File.size(path), - for_theme: true) + filename = params[:file]&.original_filename || File.basename(path) + upload = UploadCreator.new(file, filename, for_theme: true).create_for(current_user.id) if upload.errors.count > 0 render json: upload.errors, status: :unprocessable_entity else - render json: {upload_id: upload.id}, status: :created + render json: { upload_id: upload.id }, status: :created end end end def import - @theme = nil if params[:theme] json = JSON::parse(params[:theme].read) @@ -48,7 +46,6 @@ class Admin::ThemesController < Admin::AdminController else render json: @theme.errors, status: :unprocessable_entity end - end def index @@ -206,7 +203,6 @@ class Admin::ThemesController < Admin::AdminController end def set_fields - return unless fields = theme_params[:theme_fields] fields.each do |field| diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 7614b30f5db..598c1b776bd 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -1,3 +1,5 @@ +require_dependency 'upload_creator' + class UploadsController < ApplicationController before_filter :ensure_logged_in, except: [:show] skip_before_filter :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show] @@ -5,26 +7,25 @@ class UploadsController < ApplicationController def create type = params.require(:type) - raise Discourse::InvalidAccess.new unless type =~ /^[a-z\-\_]{1,100}$/ + raise Discourse::InvalidAccess.new unless Upload::UPLOAD_TYPES.include?(type) - file = params[:file] || params[:files].try(:first) - url = params[:url] - client_id = params[:client_id] - synchronous = (current_user.staff? || is_api?) && params[:synchronous] - - if type == "avatar" - if SiteSetting.sso_overrides_avatar || !SiteSetting.allow_uploaded_avatars - return render json: failed_json, status: 422 - end + if type == "avatar" && (SiteSetting.sso_overrides_avatar || !SiteSetting.allow_uploaded_avatars) + return render json: failed_json, status: 422 end - if synchronous - data = create_upload(type, file, url) + url = params[:url] + file = params[:file] || params[:files]&.first + + if params[:synchronous] && (current_user.staff? || is_api?) + data = create_upload(file, url, type) render json: data.as_json else Scheduler::Defer.later("Create Upload") do - data = create_upload(type, file, url) - MessageBus.publish("/uploads/#{type}", data.as_json, client_ids: [client_id]) + begin + data = create_upload(file, url, type) + ensure + MessageBus.publish("/uploads/#{type}", (data || {}).as_json, client_ids: [params[:client_id]]) + end end render json: success_json end @@ -58,86 +59,31 @@ class UploadsController < ApplicationController render nothing: true, status: 404 end - def create_upload(type, file, url) - begin - maximum_upload_size = [SiteSetting.max_image_size_kb, SiteSetting.max_attachment_size_kb].max.kilobytes - - # ensure we have a file - if file.nil? - # API can provide a URL - if url.present? && is_api? - tempfile = FileHelper.download(url, maximum_upload_size, "discourse-upload-#{type}") rescue nil - filename = File.basename(URI.parse(url).path) - end - else - tempfile = file.tempfile - filename = file.original_filename - content_type = file.content_type + def create_upload(file, url, type) + if file.nil? + if url.present? && is_api? + maximum_upload_size = [SiteSetting.max_image_size_kb, SiteSetting.max_attachment_size_kb].max.kilobytes + tempfile = FileHelper.download(url, maximum_upload_size, "discourse-upload-#{type}") rescue nil + filename = File.basename(URI.parse(url).path) end - - return { errors: I18n.t("upload.file_missing") } if tempfile.nil? - - # convert pasted images to HQ jpegs - if filename == "image.png" && SiteSetting.convert_pasted_images_to_hq_jpg - jpeg_path = "#{File.dirname(tempfile.path)}/image.jpg" - OptimizedImage.ensure_safe_paths!(tempfile.path, jpeg_path) - - Discourse::Utils.execute_command('convert', tempfile.path, '-quality', SiteSetting.convert_pasted_images_quality.to_s, jpeg_path) - # only change the format of the image when JPG is at least 5% smaller - if File.size(jpeg_path) < File.size(tempfile.path) * 0.95 - filename = "image.jpg" - content_type = "image/jpeg" - tempfile = File.open(jpeg_path) - else - File.delete(jpeg_path) rescue nil - end - end - - # allow users to upload large images that will be automatically reduced to allowed size - max_image_size_kb = SiteSetting.max_image_size_kb.kilobytes - if max_image_size_kb > 0 && FileHelper.is_image?(filename) - if File.size(tempfile.path) >= max_image_size_kb && Upload.should_optimize?(tempfile.path) - attempt = 2 - allow_animation = type == "avatar" ? SiteSetting.allow_animated_avatars : SiteSetting.allow_animated_thumbnails - while attempt > 0 - downsized_size = File.size(tempfile.path) - break if downsized_size < max_image_size_kb - image_info = FastImage.new(tempfile.path) rescue nil - w, h = *(image_info.try(:size) || [0, 0]) - break if w == 0 || h == 0 - downsize_ratio = best_downsize_ratio(downsized_size, max_image_size_kb) - dimensions = "#{(w * downsize_ratio).floor}x#{(h * downsize_ratio).floor}" - OptimizedImage.downsize(tempfile.path, tempfile.path, dimensions, filename: filename, allow_animation: allow_animation) - attempt -= 1 - end - end - end - - upload = Upload.create_for(current_user.id, tempfile, filename, File.size(tempfile.path), content_type: content_type, image_type: type) - - if upload.errors.empty? && current_user.admin? - retain_hours = params[:retain_hours].to_i - upload.update_columns(retain_hours: retain_hours) if retain_hours > 0 - end - - if upload.errors.empty? && FileHelper.is_image?(filename) - Jobs.enqueue(:create_thumbnails, upload_id: upload.id, type: type, user_id: params[:user_id]) - end - - upload.errors.empty? ? upload : { errors: upload.errors.values.flatten } - ensure - tempfile.try(:close!) rescue nil - end - end - - def best_downsize_ratio(downsized_size, max_image_size) - if downsized_size / 9 > max_image_size - 0.3 - elsif downsized_size / 3 > max_image_size - 0.6 else - 0.8 + tempfile = file.tempfile + filename = file.original_filename + content_type = file.content_type end + + return { errors: [I18n.t("upload.file_missing")] } if tempfile.nil? + + upload = UploadCreator.new(tempfile, filename, type: type, content_type: content_type).create_for(current_user.id) + + if upload.errors.empty? && current_user.admin? + retain_hours = params[:retain_hours].to_i + upload.update_columns(retain_hours: retain_hours) if retain_hours > 0 + end + + upload.errors.empty? ? upload : { errors: upload.errors.values.flatten } + ensure + tempfile&.close! rescue nil end end diff --git a/app/jobs/onceoff/migrate_custom_emojis.rb b/app/jobs/onceoff/migrate_custom_emojis.rb index 9eaff5190b2..72997c16133 100644 --- a/app/jobs/onceoff/migrate_custom_emojis.rb +++ b/app/jobs/onceoff/migrate_custom_emojis.rb @@ -1,3 +1,5 @@ +require_dependency 'upload_creator' + module Jobs class MigrateCustomEmojis < Jobs::Onceoff def execute_onceoff(args) @@ -7,13 +9,11 @@ module Jobs name = File.basename(path, File.extname(path)) File.open(path) do |file| - upload = Upload.create_for( - Discourse.system_user.id, + upload = UploadCreator.new( file, File.basename(path), - file.size, - image_type: 'custom_emoji' - ) + type: 'custom_emoji' + ).create_for(Discourse.system_user.id) if upload.persisted? custom_emoji = CustomEmoji.new(name: name, upload: upload) diff --git a/app/jobs/regular/create_avatar_thumbnails.rb b/app/jobs/regular/create_avatar_thumbnails.rb new file mode 100644 index 00000000000..445d32b9deb --- /dev/null +++ b/app/jobs/regular/create_avatar_thumbnails.rb @@ -0,0 +1,20 @@ +module Jobs + + class CreateAvatarThumbnails < Jobs::Base + + def execute(args) + upload_id = args[:upload_id] + + raise Discourse::InvalidParameters.new(:upload_id) if upload_id.blank? + + upload = Upload.find(upload_id) + user = User.find(args[:user_id] || upload.user_id) + + Discourse.avatar_sizes.each do |size| + OptimizedImage.create_for(upload, size, size, filename: upload.original_filename, allow_animation: SiteSetting.allow_animated_avatars) + end + end + + end + +end diff --git a/app/jobs/regular/create_thumbnails.rb b/app/jobs/regular/create_thumbnails.rb deleted file mode 100644 index 84285333d86..00000000000 --- a/app/jobs/regular/create_thumbnails.rb +++ /dev/null @@ -1,37 +0,0 @@ -module Jobs - - class CreateThumbnails < Jobs::Base - - def execute(args) - type = args[:type] - upload_id = args[:upload_id] - - raise Discourse::InvalidParameters.new(:type) if type.blank? - raise Discourse::InvalidParameters.new(:upload_id) if upload_id.blank? - - # only need to generate thumbnails for avatars - return if type != "avatar" - - upload = Upload.find(upload_id) - - user_id = args[:user_id] || upload.user_id - user = User.find(user_id) - - self.send("create_thumbnails_for_#{type}", upload, user) - end - - def create_thumbnails_for_avatar(upload, user) - Discourse.avatar_sizes.each do |size| - OptimizedImage.create_for( - upload, - size, - size, - filename: upload.original_filename, - allow_animation: SiteSetting.allow_animated_avatars - ) - end - end - - end - -end diff --git a/app/jobs/regular/pull_hotlinked_images.rb b/app/jobs/regular/pull_hotlinked_images.rb index 14bd57948e9..1044bf60269 100644 --- a/app/jobs/regular/pull_hotlinked_images.rb +++ b/app/jobs/regular/pull_hotlinked_images.rb @@ -1,5 +1,6 @@ require_dependency 'url_helper' require_dependency 'file_helper' +require_dependency 'upload_creator' module Jobs @@ -41,7 +42,7 @@ module Jobs if hotlinked if File.size(hotlinked.path) <= @max_size filename = File.basename(URI.parse(src).path) - upload = Upload.create_for(post.user_id, hotlinked, filename, File.size(hotlinked.path), { origin: src }) + upload = UploadCreator.new(hotlinked, filename, origin: src).create_for(post.user_id) downloaded_urls[src] = upload.url else Rails.logger.info("Failed to pull hotlinked image for post: #{post_id}: #{src} - Image is bigger than #{@max_size}") diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb index 7cbbee96368..60f7d471769 100644 --- a/app/models/optimized_image.rb +++ b/app/models/optimized_image.rb @@ -178,7 +178,7 @@ class OptimizedImage < ActiveRecord::Base #{from}[0] -gravity center -background transparent - -resize #{dimensions}#{!!opts[:force_aspect_ratio] ? "\\!" : "\\>"} + -resize #{dimensions} -profile #{File.join(Rails.root, 'vendor', 'data', 'RT_sRGB.icm')} #{to} } diff --git a/app/models/remote_theme.rb b/app/models/remote_theme.rb index 92e9eec2111..b426030dbea 100644 --- a/app/models/remote_theme.rb +++ b/app/models/remote_theme.rb @@ -1,4 +1,5 @@ require_dependency 'git_importer' +require_dependency 'upload_creator' class RemoteTheme < ActiveRecord::Base @@ -50,7 +51,7 @@ class RemoteTheme < ActiveRecord::Base theme_info["assets"]&.each do |name, relative_path| if path = importer.real_path(relative_path) - upload = Upload.create_for(theme.user_id, File.open(path), File.basename(relative_path), File.size(path), for_theme: true) + upload = UploadCreator.new(File.open(path), File.basename(relative_path), for_theme: true).create_for(theme.user_id) theme.set_field(target: :common, name: name, type: :theme_upload_var, upload_id: upload.id) end end diff --git a/app/models/upload.rb b/app/models/upload.rb index 97cccfa168e..318a5c822ce 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -1,5 +1,4 @@ require "digest/sha1" -require_dependency "image_sizer" require_dependency "file_helper" require_dependency "url_helper" require_dependency "db_helper" @@ -22,6 +21,9 @@ class Upload < ActiveRecord::Base validates_with ::Validators::UploadValidator + CROPPED_TYPES ||= %w{avatar card_background custom_emoji profile_background} + UPLOAD_TYPES ||= CROPPED_TYPES + %w{composer} + def thumbnail(width = self.width, height = self.height) optimized_images.find_by(width: width, height: height) end @@ -57,198 +59,10 @@ class Upload < ActiveRecord::Base File.extname(original_filename) end - # list of image types that will be cropped - CROPPED_IMAGE_TYPES ||= %w{ - avatar - profile_background - card_background - custom_emoji - } - - WHITELISTED_SVG_ELEMENTS ||= %w{ - circle - clippath - defs - ellipse - g - line - linearGradient - path - polygon - polyline - radialGradient - rect - stop - svg - text - textpath - tref - tspan - use - } - def self.generate_digest(path) Digest::SHA1.file(path).hexdigest end - def self.svg_whitelist_xpath - @@svg_whitelist_xpath ||= "//*[#{WHITELISTED_SVG_ELEMENTS.map { |e| "name()!='#{e}'" }.join(" and ") }]" - end - - # options - # - content_type - # - origin (url) - # - image_type ("avatar", "profile_background", "card_background", "custom_emoji") - # - is_attachment_for_group_message (boolean) - # - for_theme (boolean) - def self.create_for(user_id, file, filename, filesize, options = {}) - upload = Upload.new - - DistributedMutex.synchronize("upload_#{user_id}_#{filename}") do - # do some work on images - if FileHelper.is_image?(filename) && is_actual_image?(file) - # retrieve image info - w, h = FastImage.size(file) || [0, 0] - - if filename[/\.svg$/i] - # whitelist svg elements - doc = Nokogiri::XML(file) - doc.xpath(svg_whitelist_xpath).remove - File.write(file.path, doc.to_s) - file.rewind - else - if w * h >= SiteSetting.max_image_megapixels * 1_000_000 - upload.errors.add(:base, I18n.t("upload.images.larger_than_x_megapixels", max_image_megapixels: SiteSetting.max_image_megapixels)) - return upload - end - - # fix orientation first - fix_image_orientation(file.path) if should_optimize?(file.path, [w, h]) - end - - # default size - width, height = ImageSizer.resize(w, h) - - # make sure we're at the beginning of the file (both FastImage and Nokogiri move the pointer) - file.rewind - - # crop images depending on their type - if CROPPED_IMAGE_TYPES.include?(options[:image_type]) - allow_animation = SiteSetting.allow_animated_thumbnails - max_pixel_ratio = Discourse::PIXEL_RATIOS.max - - case options[:image_type] - when "avatar" - allow_animation = SiteSetting.allow_animated_avatars - width = height = Discourse.avatar_sizes.max - OptimizedImage.resize(file.path, file.path, width, height, filename: filename, allow_animation: allow_animation) - when "profile_background" - max_width = 850 * max_pixel_ratio - width, height = ImageSizer.resize(w, h, max_width: max_width, max_height: max_width) - OptimizedImage.downsize(file.path, file.path, "#{width}x#{height}", filename: filename, allow_animation: allow_animation) - when "card_background" - max_width = 590 * max_pixel_ratio - width, height = ImageSizer.resize(w, h, max_width: max_width, max_height: max_width) - OptimizedImage.downsize(file.path, file.path, "#{width}x#{height}", filename: filename, allow_animation: allow_animation) - when "custom_emoji" - OptimizedImage.downsize(file.path, file.path, "100x100", filename: filename, allow_animation: allow_animation) - end - end - - # optimize image (except GIFs, SVGs and large PNGs) - if should_optimize?(file.path, [w, h]) - begin - ImageOptim.new.optimize_image!(file.path) - rescue ImageOptim::Worker::TimeoutExceeded - # Don't optimize if it takes too long - Rails.logger.warn("ImageOptim timed out while optimizing #{filename}") - end - # update the file size - filesize = File.size(file.path) - end - end - - # compute the sha of the file - sha1 = Upload.generate_digest(file) - - # do we already have that upload? - upload = find_by(sha1: sha1) - - # make sure the previous upload has not failed - if upload && (upload.url.blank? || is_dimensionless_image?(filename, upload.width, upload.height)) - upload.destroy - upload = nil - end - - # return the previous upload if any - return upload unless upload.nil? - - # create the upload otherwise - upload = Upload.new - upload.user_id = user_id - upload.original_filename = filename - upload.filesize = filesize - upload.sha1 = sha1 - upload.url = "" - upload.width = width - upload.height = height - upload.origin = options[:origin][0...1000] if options[:origin] - - if options[:is_attachment_for_group_message] - upload.is_attachment_for_group_message = true - end - - if options[:for_theme] - upload.for_theme = true - end - - if is_dimensionless_image?(filename, upload.width, upload.height) - upload.errors.add(:base, I18n.t("upload.images.size_not_found")) - return upload - end - - return upload unless upload.save - - # store the file and update its url - File.open(file.path) do |f| - url = Discourse.store.store_upload(f, upload, options[:content_type]) - if url.present? - upload.url = url - upload.save - else - upload.errors.add(:url, I18n.t("upload.store_failure", { upload_id: upload.id, user_id: user_id })) - end - end - - upload - end - end - - def self.is_actual_image?(file) - # due to ImageMagick CVE-2016–3714, use FastImage to check the magic bytes - # cf. https://meta.discourse.org/t/imagemagick-cve-2016-3714/43624 - FastImage.size(file, raise_on_failure: true) - rescue - false - end - - LARGE_PNG_SIZE ||= 3.megabytes - - def self.should_optimize?(path, dimensions = nil) - # don't optimize GIFs or SVGs - return false if path =~ /\.(gif|svg)$/i - return true if path !~ /\.png$/i - - dimensions ||= (FastImage.size(path) || [0, 0]) - w, h = dimensions - # don't optimize large PNGs - w > 0 && h > 0 && w * h < LARGE_PNG_SIZE - end - - def self.is_dimensionless_image?(filename, width, height) - FileHelper.is_image?(filename) && (width.blank? || width == 0 || height.blank? || height == 0) - end - def self.get_from_url(url) return if url.blank? # we store relative urls, so we need to remove any host/cdn @@ -263,10 +77,6 @@ class Upload < ActiveRecord::Base Upload.find_by(url: url) end - def self.fix_image_orientation(path) - Discourse::Utils.execute_command('convert', path, '-auto-orient', path) - end - def self.migrate_to_new_scheme(limit=nil) problems = [] diff --git a/app/models/user_avatar.rb b/app/models/user_avatar.rb index 837f3ebc790..4d5b6e7d696 100644 --- a/app/models/user_avatar.rb +++ b/app/models/user_avatar.rb @@ -1,4 +1,5 @@ require_dependency 'letter_avatar' +require_dependency 'upload_creator' class UserAvatar < ActiveRecord::Base belongs_to :user @@ -20,7 +21,7 @@ class UserAvatar < ActiveRecord::Base max = Discourse.avatar_sizes.max gravatar_url = "http://www.gravatar.com/avatar/#{email_hash}.png?s=#{max}&d=404" tempfile = FileHelper.download(gravatar_url, SiteSetting.max_image_size_kb.kilobytes, "gravatar") - upload = Upload.create_for(user_id, tempfile, 'gravatar.png', File.size(tempfile.path), origin: gravatar_url, image_type: "avatar") + upload = UploadCreator.new(tempfile, 'gravatar.png', origin: gravatar_url, type: "avatar").create_for(user_id) if gravatar_upload_id != upload.id gravatar_upload.try(:destroy!) rescue nil @@ -65,7 +66,7 @@ class UserAvatar < ActiveRecord::Base ext = FastImage.type(tempfile).to_s tempfile.rewind - upload = Upload.create_for(user.id, tempfile, "external-avatar." + ext, File.size(tempfile.path), origin: avatar_url, image_type: "avatar") + upload = UploadCreator.new(tempfile, "external-avatar." + ext, origin: avatar_url, type: "avatar").create_for(user.id) user.create_user_avatar unless user.user_avatar diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 8c82eae6373..4fa51fa6e77 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1179,8 +1179,7 @@ en: allow_all_attachments_for_group_messages: "Allow all email attachments for group messages." - convert_pasted_images_to_hq_jpg: "Convert pasted images to high-quality JPG files." - convert_pasted_images_quality: "Quality of the converted JPG file (1 is lowest quality, 100 is best quality)." + png_to_jpg_quality: "Quality of the converted JPG file (1 is lowest quality, 99 is best quality, 100 to disable)." enable_flash_video_onebox: "Enable embedding of swf and flv (Adobe Flash) links in oneboxes. WARNING: may introduce security risks." @@ -2721,12 +2720,15 @@ en: deleted: 'deleted' + image: "image" + upload: edit_reason: "downloaded local copies of images" unauthorized: "Sorry, the file you are trying to upload is not authorized (authorized extensions: %{authorized_extensions})." pasted_image_filename: "Pasted image" store_failure: "Failed to store upload #%{upload_id} for user #%{user_id}." file_missing: "Sorry, you must provide a file to upload." + empty: "Sorry, but the file you provided is empty." attachments: too_large: "Sorry, the file you are trying to upload is too big (maximum size is %{max_size_kb}KB)." images: diff --git a/config/site_settings.yml b/config/site_settings.yml index c64bce3de31..75923a1778d 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -481,7 +481,7 @@ posting: default: true client: true editing_grace_period: 300 - post_edit_time_limit: + post_edit_time_limit: default: 86400 max: 525600 edit_history_visible_to_public: @@ -794,8 +794,7 @@ files: regex: '^((https?:)?\/)?\/.+[^\/]' shadowed_by_global: true allow_all_attachments_for_group_messages: false - convert_pasted_images_to_hq_jpg: true - convert_pasted_images_quality: + png_to_jpg_quality: default: 95 min: 1 max: 100 diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 810e682a14f..f1e5ec9a1e1 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -2,6 +2,7 @@ require "digest" require_dependency "new_post_manager" require_dependency "post_action_creator" require_dependency "html_to_markdown" +require_dependency "upload_creator" module Email @@ -570,7 +571,7 @@ module Email File.open(tmp.path, "w+b") { |f| f.write attachment.body.decoded } # create the upload for the user opts = { is_attachment_for_group_message: options[:is_group_message] } - upload = Upload.create_for(options[:user].id, tmp, attachment.filename, tmp.size, opts) + upload = UploadCreator.new(tmp, attachment.filename, opts).create_for(options[:user].id) if upload && upload.errors.empty? # try to inline images if attachment.content_type.start_with?("image/") diff --git a/lib/tasks/uploads.rake b/lib/tasks/uploads.rake index c281ad19194..24ac08179ca 100644 --- a/lib/tasks/uploads.rake +++ b/lib/tasks/uploads.rake @@ -143,7 +143,7 @@ def migrate_from_s3 if filename = guess_filename(url, post.raw) puts "FILENAME: #{filename}" file = FileHelper.download("http:#{url}", 20.megabytes, "from_s3", true) - if upload = Upload.create_for(post.user_id || -1, file, filename, File.size(file)) + if upload = UploadCreator.new(file, filename, File.size(file)).create_for(post.user_id || -1) post.raw = post.raw.gsub(/(https?:)?#{Regexp.escape(url)}/, upload.url) post.save post.rebake! @@ -433,7 +433,7 @@ def recover_from_tombstone if File.exists?(tombstone_path) File.open(tombstone_path) do |file| - new_upload = Upload.create_for(Discourse::SYSTEM_USER_ID, file, File.basename(url), File.size(file)) + new_upload = UploadCreator.new(file, File.basename(url), File.size(file)).create_for(Discourse::SYSTEM_USER_ID) if new_upload.persisted? printf "Restored into #{new_upload.url}\n" diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb new file mode 100644 index 00000000000..c380da175d3 --- /dev/null +++ b/lib/upload_creator.rb @@ -0,0 +1,255 @@ +require "fastimage" +require_dependency "image_sizer" + +class UploadCreator + + TYPES_CONVERTED_TO_JPEG ||= %i{bmp png} + + WHITELISTED_SVG_ELEMENTS ||= %w{ + circle clippath defs ellipse g line linearGradient path polygon polyline + radialGradient rect stop svg text textpath tref tspan use + } + + # Available options + # - type (string) + # - content_type (string) + # - origin (string) + # - is_attachment_for_group_message (boolean) + # - for_theme (boolean) + def initialize(file, filename, opts = {}) + @upload = Upload.new + @file = file + @filename = filename + @opts = opts + end + + def create_for(user_id) + if filesize <= 0 + @upload.errors.add(:base, I18n.t("upload.empty")) + return @upload + end + + DistributedMutex.synchronize("upload_#{user_id}_#{@filename}") do + if FileHelper.is_image?(@filename) + extract_image_info! + return @upload if @upload.errors.present? + + if @filename[/\.svg$/i] + whitelist_svg! + else + convert_to_jpeg! if should_convert_to_jpeg? + downsize! if should_downsize? + + return @upload if is_still_too_big? + + fix_orientation! if should_fix_orientation? + crop! if should_crop? + optimize! if should_optimize? + end + end + + # compute the sha of the file + sha1 = Upload.generate_digest(@file) + + # do we already have that upload? + @upload = Upload.find_by(sha1: sha1) + + # make sure the previous upload has not failed + if @upload && @upload.url.blank? + @upload.destroy + @upload = nil + end + + # return the previous upload if any + return @upload unless @upload.nil? + + # create the upload otherwise + @upload = Upload.new + @upload.user_id = user_id + @upload.original_filename = @filename + @upload.filesize = filesize + @upload.sha1 = sha1 + @upload.url = "" + @upload.origin = @opts[:origin][0...1000] if @opts[:origin] + + if FileHelper.is_image?(@filename) + @upload.width, @upload.height = ImageSizer.resize(*@image_info.size) + end + + if @opts[:is_attachment_for_group_message] + @upload.is_attachment_for_group_message = true + end + + if @opts[:for_theme] + @upload.for_theme = true + end + + return @upload unless @upload.save + + # store the file and update its url + File.open(@file.path) do |f| + url = Discourse.store.store_upload(f, @upload, @opts[:content_type]) + if url.present? + @upload.url = url + @upload.save + else + @upload.errors.add(:url, I18n.t("upload.store_failure", upload_id: @upload.id, user_id: user_id)) + end + end + + if @upload.errors.empty? && FileHelper.is_image?(@filename) && @opts[:type] == "avatar" + Jobs.enqueue(:create_avatar_thumbnails, upload_id: @upload.id, user_id: user_id) + end + + @upload + end + ensure + @file.close! rescue nil + end + + def extract_image_info! + @image_info = FastImage.new(@file) rescue nil + @file.rewind + + if @image_info.nil? + @upload.errors.add(:base, I18n.t("upload.images.not_supported_or_corrupted")) + elsif filesize <= 0 + @upload.errors.add(:base, I18n.t("upload.empty")) + elsif pixels == 0 + @upload.errors.add(:base, I18n.t("upload.images.size_not_found")) + end + end + + def should_convert_to_jpeg? + TYPES_CONVERTED_TO_JPEG.include?(@image_info.type) && + @image_info.size.min > 720 && + SiteSetting.png_to_jpg_quality < 100 + end + + def convert_to_jpeg! + jpeg_tempfile = Tempfile.new(["image", ".jpg"]) + + OptimizedImage.ensure_safe_paths!(@file.path, jpeg_tempfile.path) + Discourse::Utils.execute_command('convert', @file.path, '-quality', SiteSetting.png_to_jpg_quality.to_s, jpeg_tempfile.path) + + # keep the JPEG if it's at least 15% smaller + if File.size(jpeg_tempfile.path) < filesize * 0.85 + @image_info = FastImage.new(jpeg_tempfile) + @file = jpeg_tempfile + @filename = (File.basename(@filename, ".*").presence || I18n.t("image").presence || "image") + ".jpg" + @opts[:content_type] = "image/jpeg" + else + jpeg_tempfile.close! rescue nil + end + end + + def should_downsize? + max_image_size > 0 && filesize >= max_image_size + end + + def downsize! + 3.times do + original_size = filesize + downsized_pixels = [pixels, max_image_pixels].min / 2 + OptimizedImage.downsize(@file.path, @file.path, "#{downsized_pixels}@", filename: @filename, allow_animation: allow_animation) + extract_image_info! + return if filesize >= original_size || pixels == 0 || !should_downsize? + end + end + + def is_still_too_big? + if max_image_pixels > 0 && pixels >= max_image_pixels + @upload.errors.add(:base, I18n.t("upload.images.larger_than_x_megapixels", max_image_megapixels: SiteSetting.max_image_megapixels)) + true + elsif max_image_size > 0 && filesize >= max_image_size + @upload.errors.add(:base, I18n.t("upload.images.too_large", max_size_kb: SiteSetting.max_image_size_kb)) + true + else + false + end + end + + def whitelist_svg! + doc = Nokogiri::XML(@file) + doc.xpath(svg_whitelist_xpath).remove + File.write(@file.path, doc.to_s) + @file.rewind + end + + def should_crop? + Upload::CROPPED_TYPES.include?(@opts[:type]) + end + + def crop! + max_pixel_ratio = Discourse::PIXEL_RATIOS.max + + case @opts[:type] + when "avatar" + width = height = Discourse.avatar_sizes.max + OptimizedImage.resize(@file.path, @file.path, width, height, filename: @filename, allow_animation: allow_animation) + when "profile_background" + max_width = 850 * max_pixel_ratio + width, height = ImageSizer.resize(@image_info.size[0], @image_info.size[1], max_width: max_width, max_height: max_width) + OptimizedImage.downsize(@file.path, @file.path, "#{width}x#{height}\\>", filename: @filename, allow_animation: allow_animation) + when "card_background" + max_width = 590 * max_pixel_ratio + width, height = ImageSizer.resize(@image_info.size[0], @image_info.size[1], max_width: max_width, max_height: max_width) + OptimizedImage.downsize(@file.path, @file.path, "#{width}x#{height}\\>", filename: @filename, allow_animation: allow_animation) + when "custom_emoji" + OptimizedImage.downsize(@file.path, @file.path, "100x100\\>", filename: @filename, allow_animation: allow_animation) + end + end + + def should_fix_orientation? + # orientation is between 1 and 8, 1 being the default + # cf. http://www.daveperrett.com/articles/2012/07/28/exif-orientation-handling-is-a-ghetto/ + @image_info.orientation.to_i > 1 + end + + def fix_orientation! + OptimizedImage.ensure_safe_paths!(@file.path) + Discourse::Utils.execute_command('convert', @file.path, '-auto-orient', @file.path) + end + + def should_optimize? + # GIF is too slow (plus, we'll soon be converting them to MP4) + # Optimizing SVG is useless + return false if @file.path =~ /\.(gif|svg)$/i + # Safeguard for large PNGs + return pixels < 2_000_000 if @file.path =~ /\.png/i + # Everything else is fine! + true + end + + def optimize! + OptimizedImage.ensure_safe_paths!(@file.path) + ImageOptim.new.optimize_image!(@file.path) + rescue ImageOptim::Worker::TimeoutExceeded + Rails.logger.warn("ImageOptim timed out while optimizing #{@filename}") + end + + def filesize + File.size?(@file.path).to_i + end + + def max_image_size + @@max_image_size ||= SiteSetting.max_image_size_kb.kilobytes + end + + def max_image_pixels + @@max_image_pixels ||= SiteSetting.max_image_megapixels * 1_000_000 + end + + def pixels + @image_info.size&.reduce(:*).to_i + end + + def allow_animation + @@allow_animation ||= @opts[:type] == "avatar" ? SiteSetting.allow_animated_avatars : SiteSetting.allow_animated_thumbnails + end + + def svg_whitelist_xpath + @@svg_whitelist_xpath ||= "//*[#{WHITELISTED_SVG_ELEMENTS.map { |e| "name()!='#{e}'" }.join(" and ") }]" + end + +end diff --git a/script/import_scripts/base/uploader.rb b/script/import_scripts/base/uploader.rb index 62ddac451d7..ca7b65bb1dc 100644 --- a/script/import_scripts/base/uploader.rb +++ b/script/import_scripts/base/uploader.rb @@ -15,7 +15,7 @@ module ImportScripts src.close tmp.rewind - Upload.create_for(user_id, tmp, source_filename, tmp.size) + UploadCreator.new(tmp, source_filename).create_for(user_id) rescue => e Rails.logger.error("Failed to create upload: #{e}") nil diff --git a/script/import_scripts/lithium.rb b/script/import_scripts/lithium.rb index 1591cb8f2b6..bd0a6f00553 100644 --- a/script/import_scripts/lithium.rb +++ b/script/import_scripts/lithium.rb @@ -144,7 +144,7 @@ class ImportScripts::Lithium < ImportScripts::Base file.write(picture["filedata"].encode("ASCII-8BIT").force_encoding("UTF-8")) file.rewind - upload = Upload.create_for(imported_user.id, file, picture["filename"], file.size) + upload = UploadCreator.new(file, picture["filename"]).create_for(imported_user.id) return if !upload.persisted? @@ -173,7 +173,7 @@ class ImportScripts::Lithium < ImportScripts::Base file.write(background["filedata"].encode("ASCII-8BIT").force_encoding("UTF-8")) file.rewind - upload = Upload.create_for(imported_user.id, file, background["filename"], file.size) + upload = UploadCreator.new(file, background["filename"]).create_for(imported_user.id) return if !upload.persisted? @@ -807,7 +807,7 @@ SQL if image File.open(image) do |file| - upload = Upload.create_for(user_id, file, "image." + (image =~ /.png$/ ? "png": "jpg"), File.size(image)) + upload = UploadCreator.new(file, "image." + (image.ends_with?(".png") ? "png" : "jpg")).create_for(user_id) l["src"] = upload.url end else diff --git a/script/import_scripts/mbox.rb b/script/import_scripts/mbox.rb index dce24c54027..e7c25ed18c6 100755 --- a/script/import_scripts/mbox.rb +++ b/script/import_scripts/mbox.rb @@ -439,7 +439,7 @@ p end # read attachment File.open(tmp.path, "w+b") { |f| f.write attachment.body.decoded } # create the upload for the user - upload = Upload.create_for(user_id_from_imported_user_id(from_email) || Discourse::SYSTEM_USER_ID, tmp, attachment.filename, tmp.size ) + upload = UploadCreator.new(tmp, attachment.filename).create_for(user_id_from_imported_user_id(from_email) || Discourse::SYSTEM_USER_ID) if upload && upload.errors.empty? raw << "\n\n#{receiver.attachment_markdown(upload)}\n\n" end @@ -530,7 +530,7 @@ p end # read attachment File.open(tmp.path, "w+b") { |f| f.write attachment.body.decoded } # create the upload for the user - upload = Upload.create_for(user_id_from_imported_user_id(from_email) || Discourse::SYSTEM_USER_ID, tmp, attachment.filename, tmp.size ) + upload = UploadCreator.new(tmp, attachment.filename).create_for(user_id_from_imported_user_id(from_email) || Discourse::SYSTEM_USER_ID) if upload && upload.errors.empty? raw << "\n\n#{receiver.attachment_markdown(upload)}\n\n" end diff --git a/script/import_scripts/sfn.rb b/script/import_scripts/sfn.rb index 14e54196cdb..64a1dc54a96 100644 --- a/script/import_scripts/sfn.rb +++ b/script/import_scripts/sfn.rb @@ -101,7 +101,7 @@ class ImportScripts::Sfn < ImportScripts::Base avatar.write(user["avatar"].encode("ASCII-8BIT").force_encoding("UTF-8")) avatar.rewind - upload = Upload.create_for(newuser.id, avatar, "avatar.jpg", avatar.size) + upload = UploadCreator.new(avatar, "avatar.jpg").create_for(newuser.id) if upload.persisted? newuser.create_user_avatar newuser.user_avatar.update(custom_upload_id: upload.id) diff --git a/script/import_scripts/vbulletin.rb b/script/import_scripts/vbulletin.rb index f7733d60c9a..0ed2bd6a91f 100644 --- a/script/import_scripts/vbulletin.rb +++ b/script/import_scripts/vbulletin.rb @@ -201,7 +201,7 @@ EOM file.write(picture["filedata"].encode("ASCII-8BIT").force_encoding("UTF-8")) file.rewind - upload = Upload.create_for(imported_user.id, file, picture["filename"], file.size) + upload = UploadCreator.new(file, picture["filename"]).create_for(imported_user.id) return if !upload.persisted? @@ -231,7 +231,7 @@ EOM file.write(background["filedata"].encode("ASCII-8BIT").force_encoding("UTF-8")) file.rewind - upload = Upload.create_for(imported_user.id, file, background["filename"], file.size) + upload = UploadCreator.new(file, background["filename"]).create_for(imported_user.id) return if !upload.persisted? diff --git a/script/import_scripts/vbulletin5.rb b/script/import_scripts/vbulletin5.rb index 21a9f851f2b..263cfb4f48f 100644 --- a/script/import_scripts/vbulletin5.rb +++ b/script/import_scripts/vbulletin5.rb @@ -9,7 +9,7 @@ class ImportScripts::VBulletin < ImportScripts::Base # CHANGE THESE BEFORE RUNNING THE IMPORTER DATABASE = "yourforum" - TIMEZONE = "America/Los_Angeles" + TIMEZONE = "America/Los_Angeles" ATTACHMENT_DIR = '/home/discourse/yourforum/customattachments/' AVATAR_DIR = '/home/discourse/yourforum/avatars/' @@ -25,7 +25,7 @@ class ImportScripts::VBulletin < ImportScripts::Base @client = Mysql2::Client.new( host: "localhost", username: "root", - database: DATABASE, + database: DATABASE, password: "password" ) @@ -123,7 +123,7 @@ class ImportScripts::VBulletin < ImportScripts::Base file = Tempfile.new("profile-picture") file.write(picture["filedata"].encode("ASCII-8BIT").force_encoding("UTF-8")) file.rewind - upload = Upload.create_for(imported_user.id, file, picture["filename"], file.size) + upload = UploadCreator.new(file, picture["filename"]).create_for(imported_user.id) else filename = File.join(AVATAR_DIR, picture['filename']) unless File.exists?(filename) @@ -160,7 +160,7 @@ class ImportScripts::VBulletin < ImportScripts::Base file.write(background["filedata"].encode("ASCII-8BIT").force_encoding("UTF-8")) file.rewind - upload = Upload.create_for(imported_user.id, file, background["filename"], file.size) + upload = UploadCreator.new(file, background["filename"]).create_for(imported_user.id) return if !upload.persisted? @@ -173,13 +173,13 @@ class ImportScripts::VBulletin < ImportScripts::Base def import_categories puts "", "importing top level categories..." - categories = mysql_query("SELECT nodeid AS forumid, title, description, displayorder, parentid - FROM #{DBPREFIX}node - WHERE parentid=#{ROOT_NODE} - UNION - SELECT nodeid, title, description, displayorder, parentid - FROM #{DBPREFIX}node - WHERE contenttypeid = 23 + categories = mysql_query("SELECT nodeid AS forumid, title, description, displayorder, parentid + FROM #{DBPREFIX}node + WHERE parentid=#{ROOT_NODE} + UNION + SELECT nodeid, title, description, displayorder, parentid + FROM #{DBPREFIX}node + WHERE contenttypeid = 23 AND parentid IN (SELECT nodeid FROM #{DBPREFIX}node WHERE parentid=#{ROOT_NODE})").to_a top_level_categories = categories.select { |c| c["parentid"] == ROOT_NODE } @@ -222,17 +222,17 @@ class ImportScripts::VBulletin < ImportScripts::Base # keep track of closed topics @closed_topic_ids = [] - topic_count = mysql_query("select count(nodeid) cnt from #{DBPREFIX}node where parentid in ( + topic_count = mysql_query("select count(nodeid) cnt from #{DBPREFIX}node where parentid in ( select nodeid from #{DBPREFIX}node where contenttypeid=23 ) and contenttypeid=22;").first["cnt"] batches(BATCH_SIZE) do |offset| topics = mysql_query <<-SQL SELECT t.nodeid AS threadid, t.title, t.parentid AS forumid,t.open,t.userid AS postuserid,t.publishdate AS dateline, - nv.count views, 1 AS visible, t.sticky, + nv.count views, 1 AS visible, t.sticky, CONVERT(CAST(rawtext AS BINARY)USING utf8) AS raw - FROM #{DBPREFIX}node t - LEFT JOIN #{DBPREFIX}nodeview nv ON nv.nodeid=t.nodeid - LEFT JOIN #{DBPREFIX}text txt ON txt.nodeid=t.nodeid + FROM #{DBPREFIX}node t + LEFT JOIN #{DBPREFIX}nodeview nv ON nv.nodeid=t.nodeid + LEFT JOIN #{DBPREFIX}text txt ON txt.nodeid=t.nodeid WHERE t.parentid in ( select nodeid from #{DBPREFIX}node where contenttypeid=23 ) AND t.contenttypeid = 22 ORDER BY t.nodeid @@ -275,17 +275,17 @@ class ImportScripts::VBulletin < ImportScripts::Base rescue end - post_count = mysql_query("SELECT COUNT(nodeid) cnt FROM #{DBPREFIX}node WHERE parentid NOT IN ( + post_count = mysql_query("SELECT COUNT(nodeid) cnt FROM #{DBPREFIX}node WHERE parentid NOT IN ( SELECT nodeid FROM #{DBPREFIX}node WHERE contenttypeid=23 ) AND contenttypeid=22;").first["cnt"] batches(BATCH_SIZE) do |offset| posts = mysql_query <<-SQL - SELECT p.nodeid AS postid, p.userid AS userid, p.parentid AS threadid, + SELECT p.nodeid AS postid, p.userid AS userid, p.parentid AS threadid, CONVERT(CAST(rawtext AS BINARY)USING utf8) AS raw, p.publishdate AS dateline, 1 AS visible, p.parentid AS parentid - FROM #{DBPREFIX}node p - LEFT JOIN #{DBPREFIX}nodeview nv ON nv.nodeid=p.nodeid - LEFT JOIN #{DBPREFIX}text txt ON txt.nodeid=p.nodeid + FROM #{DBPREFIX}node p + LEFT JOIN #{DBPREFIX}nodeview nv ON nv.nodeid=p.nodeid + LEFT JOIN #{DBPREFIX}text txt ON txt.nodeid=p.nodeid WHERE p.parentid NOT IN ( select nodeid from #{DBPREFIX}node where contenttypeid=23 ) AND p.contenttypeid = 22 ORDER BY postid @@ -299,7 +299,7 @@ class ImportScripts::VBulletin < ImportScripts::Base # next if all_records_exist? :posts, posts.map {|p| p["postid"] } create_posts(posts, total: post_count, offset: offset) do |post| - raw = preprocess_post_raw(post["raw"]) + raw = preprocess_post_raw(post["raw"]) next if raw.blank? next unless topic = topic_lookup_from_imported_post_id("thread-#{post["threadid"]}") p = { @@ -336,7 +336,7 @@ class ImportScripts::VBulletin < ImportScripts::Base real_filename.prepend SecureRandom.hex if real_filename[0] == '.' unless File.exists?(filename) - if row['dbsize'].to_i == 0 + if row['dbsize'].to_i == 0 puts "Attachment file #{row['filedataid']} doesn't exist" return nil end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index dc1b11c6034..8a9a7941b89 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -48,7 +48,7 @@ describe UploadsController do end it 'is successful with an image' do - Jobs.expects(:enqueue).with(:create_thumbnails, anything) + Jobs.expects(:enqueue).with(:create_avatar_thumbnails, anything) message = MessageBus.track_publish do xhr :post, :create, file: logo, type: "avatar" @@ -78,7 +78,7 @@ describe UploadsController do SiteSetting.authorized_extensions = "*" controller.stubs(:is_api?).returns(true) - Jobs.expects(:enqueue).with(:create_thumbnails, anything) + Jobs.expects(:enqueue).with(:create_avatar_thumbnails, anything) stub_request(:get, "http://example.com/image.png").to_return(body: File.read('spec/fixtures/images/logo.png')) @@ -92,7 +92,7 @@ describe UploadsController do it 'correctly sets retain_hours for admins' do log_in :admin - Jobs.expects(:enqueue).with(:create_thumbnails, anything) + Jobs.expects(:enqueue).with(:create_avatar_thumbnails, anything).never message = MessageBus.track_publish do xhr :post, :create, file: logo, retain_hours: 100, type: "profile_background" @@ -110,7 +110,7 @@ describe UploadsController do end.first expect(response.status).to eq 200 - expect(message.data["errors"]).to eq(I18n.t("upload.file_missing")) + expect(message.data["errors"]).to contain_exactly(I18n.t("upload.file_missing")) end it 'properly returns errors' do @@ -139,7 +139,7 @@ describe UploadsController do end it 'returns an error when it could not determine the dimensions of an image' do - Jobs.expects(:enqueue).with(:create_thumbnails, anything).never + Jobs.expects(:enqueue).with(:create_avatar_thumbnails, anything).never message = MessageBus.track_publish do xhr :post, :create, file: fake_jpg, type: "composer" @@ -148,8 +148,7 @@ describe UploadsController do expect(response.status).to eq 200 expect(message.channel).to eq("/uploads/composer") - expect(message.data["errors"]).to be - expect(message.data["errors"][0]).to eq(I18n.t("upload.images.size_not_found")) + expect(message.data["errors"]).to contain_exactly(I18n.t("upload.images.size_not_found")) end end diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index 14505384415..5df2a2abf72 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -171,7 +171,7 @@ HTML it 'can handle uploads based of ThemeField' do theme = Theme.new(name: 'theme', user_id: -1) - upload = Upload.create_for(-1, image, "logo.png", File.size(image)) + upload = UploadCreator.new(image, "logo.png").create_for(-1) theme.set_field(target: :common, name: :logo, upload_id: upload.id, type: :theme_upload_var) theme.set_field(target: :common, name: :scss, value: 'body {background-image: url($logo)}') theme.save! diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 4cf0ad64e05..c187ddbe675 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -46,91 +46,6 @@ describe Upload do end - context "#create_for" do - - before do - Upload.stubs(:fix_image_orientation) - ImageOptim.any_instance.stubs(:optimize_image!) - end - - it "does not create another upload if it already exists" do - Upload.expects(:find_by).with(sha1: image_sha1).returns(upload) - Upload.expects(:save).never - expect(Upload.create_for(user_id, image, image_filename, image_filesize)).to eq(upload) - end - - it "ensures images isn't huge before processing it" do - Upload.expects(:fix_image_orientation).never - upload = Upload.create_for(user_id, huge_image, huge_image_filename, huge_image_filesize) - expect(upload.errors.size).to be > 0 - end - - it "fix image orientation" do - Upload.expects(:fix_image_orientation).with(image.path) - Upload.create_for(user_id, image, image_filename, image_filesize) - end - - it "computes width & height for images" do - ImageSizer.expects(:resize) - Upload.create_for(user_id, image, image_filename, image_filesize) - end - - it "does not compute width & height for non-image" do - FastImage.any_instance.expects(:size).never - upload = Upload.create_for(user_id, attachment, attachment_filename, attachment_filesize) - expect(upload.errors.size).to be > 0 - end - - it "generates an error when the image is too large" do - SiteSetting.stubs(:max_image_size_kb).returns(1) - upload = Upload.create_for(user_id, image, image_filename, image_filesize) - expect(upload.errors.size).to be > 0 - end - - it "generates an error when the attachment is too large" do - SiteSetting.stubs(:max_attachment_size_kb).returns(1) - upload = Upload.create_for(user_id, attachment, attachment_filename, attachment_filesize) - expect(upload.errors.size).to be > 0 - end - - it "saves proper information" do - store = {} - Discourse.expects(:store).returns(store) - store.expects(:store_upload).returns(url) - - upload = Upload.create_for(user_id, image, image_filename, image_filesize) - - expect(upload.user_id).to eq(user_id) - expect(upload.original_filename).to eq(image_filename) - expect(upload.filesize).to eq(image_filesize) - expect(upload.width).to eq(244) - expect(upload.height).to eq(66) - expect(upload.url).to eq(url) - end - - context "when svg is authorized" do - - before { SiteSetting.stubs(:authorized_extensions).returns("svg") } - - it "consider SVG as an image" do - store = {} - Discourse.expects(:store).returns(store) - store.expects(:store_upload).returns(url) - - upload = Upload.create_for(user_id, image_svg, image_svg_filename, image_svg_filesize) - - expect(upload.user_id).to eq(user_id) - expect(upload.original_filename).to eq(image_svg_filename) - expect(upload.filesize).to eq(image_svg_filesize) - expect(upload.width).to eq(100) - expect(upload.height).to eq(50) - expect(upload.url).to eq(url) - end - - end - - end - context ".get_from_url" do let(:url) { "/uploads/default/original/3X/1/0/10f73034616a796dfd70177dc54b6def44c4ba6f.png" } let(:upload) { Fabricate(:upload, url: url) }