From 542d54e6bfef6672ff732d3d5d4661fac53a5064 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Tue, 15 Apr 2014 13:04:14 +0200 Subject: [PATCH] BUGFIX: uploads to S3 --- app/controllers/uploads_controller.rb | 2 +- app/jobs/regular/pull_hotlinked_images.rb | 2 +- app/models/upload.rb | 4 ++-- lib/file_helper.rb | 6 +++--- lib/file_store/base_store.rb | 2 +- lib/file_store/local_store.rb | 2 +- lib/file_store/s3_store.rb | 16 +++++----------- spec/components/file_store/s3_store_spec.rb | 21 +++------------------ 8 files changed, 17 insertions(+), 38 deletions(-) diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 02cb0c6cb0d..ec0c2783eb7 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -6,7 +6,7 @@ class UploadsController < ApplicationController file = params[:file] || params[:files].first filesize = File.size(file.tempfile) - upload = Upload.create_for(current_user.id, file.tempfile, file.original_filename, filesize) + upload = Upload.create_for(current_user.id, file.tempfile, file.original_filename, filesize, file.content_type) if upload.errors.empty? render_serialized(upload, UploadSerializer, root: false) diff --git a/app/jobs/regular/pull_hotlinked_images.rb b/app/jobs/regular/pull_hotlinked_images.rb index 078dcfa482f..39ce1c79435 100644 --- a/app/jobs/regular/pull_hotlinked_images.rb +++ b/app/jobs/regular/pull_hotlinked_images.rb @@ -34,7 +34,7 @@ module Jobs hotlinked = FileHelper.download(src, @max_size, "discourse-hotlinked") rescue Discourse::InvalidParameters if hotlinked.try(:size) <= @max_size filename = File.basename(URI.parse(src).path) - upload = Upload.create_for(post.user_id, hotlinked, filename, hotlinked.size, src) + upload = Upload.create_for(post.user_id, hotlinked, filename, hotlinked.size, nil, src) downloaded_urls[src] = upload.url else Rails.logger.error("Failed to pull hotlinked image: #{src} - Image is bigger than #{@max_size}") diff --git a/app/models/upload.rb b/app/models/upload.rb index c05b5d868b8..e91fdd9ac30 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -46,7 +46,7 @@ class Upload < ActiveRecord::Base File.extname(original_filename) end - def self.create_for(user_id, file, filename, filesize, origin = nil) + def self.create_for(user_id, file, filename, filesize, content_type = nil, origin = nil) # compute the sha sha1 = Digest::SHA1.file(file).hexdigest # check if the file has already been uploaded @@ -93,7 +93,7 @@ class Upload < ActiveRecord::Base return upload unless upload.save # store the file and update its url - url = Discourse.store.store_upload(file, upload) + url = Discourse.store.store_upload(file, upload, content_type) if url.present? upload.url = url upload.save diff --git a/lib/file_helper.rb b/lib/file_helper.rb index 77fc67500b7..41dde67f19f 100644 --- a/lib/file_helper.rb +++ b/lib/file_helper.rb @@ -11,11 +11,11 @@ class FileHelper tmp = Tempfile.new([tmp_file_name, extension]) File.open(tmp.path, "wb") do |f| - avatar = open(url, "rb", read_timeout: 5) - while f.size <= max_file_size && data = avatar.read(max_file_size) + downloaded = open(url, "rb", read_timeout: 5) + while f.size <= max_file_size && data = downloaded.read(max_file_size) f.write(data) end - avatar.close! + downloaded.close! end tmp diff --git a/lib/file_store/base_store.rb b/lib/file_store/base_store.rb index 5bbf3a43aa6..0769a52e648 100644 --- a/lib/file_store/base_store.rb +++ b/lib/file_store/base_store.rb @@ -2,7 +2,7 @@ module FileStore class BaseStore - def store_upload(file, upload) + def store_upload(file, upload, content_type = nil) end def store_optimized_image(file, optimized_image) diff --git a/lib/file_store/local_store.rb b/lib/file_store/local_store.rb index 78fd8755c45..1c7fd035d85 100644 --- a/lib/file_store/local_store.rb +++ b/lib/file_store/local_store.rb @@ -4,7 +4,7 @@ module FileStore class LocalStore < BaseStore - def store_upload(file, upload) + def store_upload(file, upload, content_type = nil) path = get_path_for_upload(file, upload) store_file(file, path) end diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index 1deb834d6cc..06fb56e1d8e 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -1,13 +1,14 @@ require 'file_store/base_store' +require_dependency "file_helper" module FileStore class S3Store < BaseStore @fog_loaded ||= require 'fog' - def store_upload(file, upload) + def store_upload(file, upload, content_type = nil) path = get_path_for_upload(file, upload) - store_file(file, path, upload.original_filename, file.content_type) + store_file(file, path, upload.original_filename, content_type) end def store_optimized_image(file, optimized_image) @@ -45,17 +46,10 @@ module FileStore end def download(upload) - @open_uri_loaded ||= require 'open-uri' - - extension = File.extname(upload.original_filename) - temp_file = Tempfile.new(["discourse-s3", extension]) url = SiteSetting.scheme + ":" + upload.url + max_file_size = [SiteSetting.max_image_size_kb, SiteSetting.max_attachment_size_kb].max.kilobytes - File.open(temp_file.path, "wb") do |f| - f.write(open(url, "rb", read_timeout: 5).read) - end - - temp_file + FileHelper.download(url, max_file_size, "discourse-s3") end def avatar_template(avatar) diff --git a/spec/components/file_store/s3_store_spec.rb b/spec/components/file_store/s3_store_spec.rb index d47c7f69c42..eff8b6e0b9d 100644 --- a/spec/components/file_store/s3_store_spec.rb +++ b/spec/components/file_store/s3_store_spec.rb @@ -7,28 +7,13 @@ describe FileStore::S3Store do let(:store) { FileStore::S3Store.new } let(:upload) { build(:upload) } - let(:uploaded_file) do - ActionDispatch::Http::UploadedFile.new({ - filename: 'logo.png', - tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") - }) - end + let(:uploaded_file) { File.new("#{Rails.root}/spec/fixtures/images/logo.png") } let(:optimized_image) { build(:optimized_image) } - let(:optimized_image_file) do - ActionDispatch::Http::UploadedFile.new({ - filename: 'logo.png', - tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") - }) - end + let(:optimized_image_file) { File.new("#{Rails.root}/spec/fixtures/images/logo.png") } let(:avatar) { build(:upload) } - let(:avatar_file) do - ActionDispatch::Http::UploadedFile.new({ - filename: 'logo-dev.png', - tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo-dev.png") - }) - end + let(:avatar_file) { File.new("#{Rails.root}/spec/fixtures/images/logo-dev.png") } before(:each) do SiteSetting.stubs(:s3_upload_bucket).returns("S3_Upload_Bucket")