Skip to content

Commit

Permalink
BUGFIX: uploads to S3
Browse files Browse the repository at this point in the history
  • Loading branch information
ZogStriP committed Apr 15, 2014
1 parent d08973d commit 542d54e
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 38 deletions.
2 changes: 1 addition & 1 deletion app/controllers/uploads_controller.rb
Expand Up @@ -6,7 +6,7 @@ def create
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)
Expand Down
2 changes: 1 addition & 1 deletion app/jobs/regular/pull_hotlinked_images.rb
Expand Up @@ -34,7 +34,7 @@ def execute(args)
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}")
Expand Down
4 changes: 2 additions & 2 deletions app/models/upload.rb
Expand Up @@ -46,7 +46,7 @@ def extension
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)

This comment has been minimized.

Copy link
@eviltrout

eviltrout Apr 15, 2014

Contributor

I find it's a good idea to use an options hash when you have multiple nilable parameters. Otherwise it gets super confusing.

This comment has been minimized.

Copy link
@ZogStriP

ZogStriP Apr 15, 2014

Author Member

good point

# compute the sha
sha1 = Digest::SHA1.file(file).hexdigest
# check if the file has already been uploaded
Expand Down Expand Up @@ -93,7 +93,7 @@ def self.create_for(user_id, file, filename, filesize, origin = nil)
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
Expand Down
6 changes: 3 additions & 3 deletions lib/file_helper.rb
Expand Up @@ -11,11 +11,11 @@ def self.download(url, max_file_size, tmp_file_name)
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
Expand Down
2 changes: 1 addition & 1 deletion lib/file_store/base_store.rb
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/file_store/local_store.rb
Expand Up @@ -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
Expand Down
16 changes: 5 additions & 11 deletions 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)
Expand Down Expand Up @@ -45,17 +46,10 @@ def internal?
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)
Expand Down
21 changes: 3 additions & 18 deletions spec/components/file_store/s3_store_spec.rb
Expand Up @@ -7,28 +7,13 @@
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")
Expand Down

0 comments on commit 542d54e

Please sign in to comment.