diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 4a40aa492f3..c46dde316a3 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -4,10 +4,14 @@ class UploadsController < ApplicationController def create requires_parameter(:topic_id) file = params[:file] || params[:files].first + # only supports images for now return render status: 415, json: failed_json unless file.content_type =~ /^image\/.+/ + upload = Upload.create_for(current_user.id, file, params[:topic_id]) + render_serialized(upload, UploadSerializer, root: false) + rescue FastImage::ImageFetchFailure render status: 422, text: I18n.t("upload.image.fetch_failure") rescue FastImage::UnknownImageType @@ -15,4 +19,5 @@ class UploadsController < ApplicationController rescue FastImage::SizeNotFound render status: 422, text: I18n.t("upload.image.size_not_found") end + end diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 829993e388f..76cfe1e8a2e 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -73,8 +73,7 @@ class SiteSetting < ActiveRecord::Base setting(:crawl_images, !Rails.env.test?) setting(:enable_imgur, false) setting(:imgur_client_id, '') - setting(:imgur_client_secret, '') - setting(:imgur_endpoint, "http://api.imgur.com/3/image.json") + setting(:imgur_endpoint, "https://api.imgur.com/3/image.json") setting(:max_image_width, 690) client_setting(:category_featured_topics, 6) setting(:topics_per_page, 30) @@ -165,7 +164,7 @@ class SiteSetting < ActiveRecord::Base setting(:enable_s3_uploads, false) setting(:s3_access_key_id, '') setting(:s3_secret_access_key, '') - setting(:s3_region, 'us-west-1') + setting(:s3_region, '') setting(:s3_upload_bucket, '') setting(:default_trust_level, 0) diff --git a/app/models/upload.rb b/app/models/upload.rb index b14442dc17d..de7226a5334 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -1,4 +1,8 @@ require 'digest/sha1' +require 'image_sizer' +require 'imgur' +require 's3' +require 'local_store' class Upload < ActiveRecord::Base belongs_to :user @@ -7,96 +11,39 @@ class Upload < ActiveRecord::Base validates_presence_of :filesize validates_presence_of :original_filename - # Create an upload given a user, file and topic def self.create_for(user_id, file, topic_id) - return create_on_imgur(user_id, file, topic_id) if SiteSetting.enable_imgur? - return create_on_s3(user_id, file, topic_id) if SiteSetting.enable_s3_uploads? - return create_locally(user_id, file, topic_id) - end + # retrieve image info + image_info = FastImage.new(file.tempfile, raise_on_failure: true) + # compute image aspect ratio + width, height = ImageSizer.resize(*image_info.size) - # Store uploads on imgur - def self.create_on_imgur(user_id, file, topic_id) - @imgur_loaded = require 'imgur' unless @imgur_loaded - - info = Imgur.upload_file(file) - - Upload.create!({ - user_id: user_id, - topic_id: topic_id, - original_filename: file.original_filename - }.merge!(info)) - end - - # Store uploads on s3 - def self.create_on_s3(user_id, file, topic_id) - @fog_loaded = require 'fog' unless @fog_loaded - - tempfile = file.tempfile - - upload = Upload.new(user_id: user_id, - topic_id: topic_id, - filesize: File.size(tempfile), - original_filename: file.original_filename) - - image_info = FastImage.new(tempfile, raise_on_failure: true) - blob = file.read - sha1 = Digest::SHA1.hexdigest(blob) - remote_filename = "#{sha1}.#{image_info.type}" - - fog = Fog::Storage.new( - aws_access_key_id: SiteSetting.s3_access_key_id, - aws_secret_access_key: SiteSetting.s3_secret_access_key, - region: SiteSetting.s3_region, - provider: 'AWS' - ) - - directory = fog.directories.create(key: SiteSetting.s3_upload_bucket) - - file = directory.files.create( - key: remote_filename, - body: tempfile, - public: true, - content_type: file.content_type - ) - - upload.width, upload.height = ImageSizer.resize(*image_info.size) - upload.url = "//#{SiteSetting.s3_upload_bucket}.s3-#{SiteSetting.s3_region}.amazonaws.com/#{remote_filename}" - - upload.save - - upload - end - - def self.create_locally(user_id, file, topic_id) upload = Upload.create!({ user_id: user_id, topic_id: topic_id, - url: "", + original_filename: file.original_filename, filesize: File.size(file.tempfile), - original_filename: file.original_filename + width: width, + height: height, + url: "" }) - # populate the rest of the info - clean_name = Digest::SHA1.hexdigest("#{Time.now.to_s}#{file.original_filename}")[0,16] - image_info = FastImage.new(file.tempfile, raise_on_failure: true) - clean_name += ".#{image_info.type}" - url_root = "/uploads/#{RailsMultisite::ConnectionManagement.current_db}/#{upload.id}" - path = "#{Rails.root}/public#{url_root}" + # make sure we're at the beginning of the file (FastImage is moving the pointer) + file.rewind - FileUtils.mkdir_p path - # not using cause mv, cause permissions are no good on move - File.open("#{path}/#{clean_name}", "wb") do |f| - f.write File.read(file.tempfile) - end - - upload.width, upload.height = ImageSizer.resize(*image_info.size) - upload.url = Discourse::base_uri + "#{url_root}/#{clean_name}" + # store the file and update its url + upload.url = Upload.store_file(file, image_info, upload.id) upload.save upload end + def self.store_file(file, image_info, upload_id) + return Imgur.store_file(file, image_info, upload_id) if SiteSetting.enable_imgur? + return S3.store_file(file, image_info, upload_id) if SiteSetting.enable_s3_uploads? + return LocalStore.store_file(file, image_info, upload_id) + end + end # == Schema Information @@ -119,4 +66,3 @@ end # index_uploads_on_forum_thread_id (topic_id) # index_uploads_on_user_id (user_id) # - diff --git a/lib/discourse.rb b/lib/discourse.rb index c787ffb9cbb..533d6a46506 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -14,6 +14,9 @@ module Discourse # When something they want is not found class NotFound < Exception; end + # When a setting is missing + class SiteSettingMissing < Exception; end + def self.cache @cache ||= Cache.new end diff --git a/lib/imgur.rb b/lib/imgur.rb index 4cc029553ab..68dc91b6d30 100644 --- a/lib/imgur.rb +++ b/lib/imgur.rb @@ -1,27 +1,25 @@ require 'rest_client' -require 'image_size' module Imgur - def self.upload_file(file) + def self.store_file(file, image_info, upload_id) + raise Discourse::SiteSettingMissing.new("imgur_endpoint") if SiteSetting.imgur_endpoint.blank? + raise Discourse::SiteSettingMissing.new("imgur_client_id") if SiteSetting.imgur_client_id.blank? + + @imgur_loaded = require 'imgur' unless @imgur_loaded blob = file.read - response = RestClient.post(SiteSetting.imgur_endpoint, { image: Base64.encode64(blob) }, { 'Authorization' => "Client-ID #{SiteSetting.imgur_client_id}" }) + + response = RestClient.post( + SiteSetting.imgur_endpoint, + { image: Base64.encode64(blob) }, + { 'Authorization' => "Client-ID #{SiteSetting.imgur_client_id}" } + ) json = JSON.parse(response.body)['data'] rescue nil return nil if json.blank? - - # Resize the image - image_info = FastImage.new(file, raise_on_failure: true) - width, height = ImageSizer.resize(*image_info.size) - - { - url: json['link'], - filesize: File.size(file), - width: width, - height: height - } + return json['link'] end end diff --git a/lib/local_store.rb b/lib/local_store.rb new file mode 100644 index 00000000000..b08d3b71fc4 --- /dev/null +++ b/lib/local_store.rb @@ -0,0 +1,18 @@ +module LocalStore + + def self.store_file(file, image_info, upload_id) + clean_name = Digest::SHA1.hexdigest("#{Time.now.to_s}#{file.original_filename}")[0,16] + ".#{image_info.type}" + url_root = "/uploads/#{RailsMultisite::ConnectionManagement.current_db}/#{upload_id}" + path = "#{Rails.root}/public#{url_root}" + + FileUtils.mkdir_p path + # not using cause mv, cause permissions are no good on move + File.open("#{path}/#{clean_name}", "wb") do |f| + f.write File.read(file.tempfile) + end + + # url + return Discourse::base_uri + "#{url_root}/#{clean_name}" + end + +end \ No newline at end of file diff --git a/lib/s3.rb b/lib/s3.rb new file mode 100644 index 00000000000..c999a54b297 --- /dev/null +++ b/lib/s3.rb @@ -0,0 +1,50 @@ +module S3 + + def self.store_file(file, image_info, upload_id) + raise Discourse::SiteSettingMissing.new("s3_upload_bucket") if SiteSetting.s3_upload_bucket.blank? + raise Discourse::SiteSettingMissing.new("s3_access_key_id") if SiteSetting.s3_access_key_id.blank? + raise Discourse::SiteSettingMissing.new("s3_secret_access_key") if SiteSetting.s3_secret_access_key.blank? + + @fog_loaded = require 'fog' unless @fog_loaded + + blob = file.read + sha1 = Digest::SHA1.hexdigest(blob) + remote_filename = "#{upload_id}#{sha1}.#{image_info.type}" + + options = S3.generate_options + directory = S3.get_or_create_directory(SiteSetting.s3_upload_bucket, options) + # if this fails, it will throw an exception + file = S3.upload(file, remote_filename, directory) + + return "//#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/#{remote_filename}" + end + + def self.generate_options + options = { + provider: 'AWS', + aws_access_key_id: SiteSetting.s3_access_key_id, + aws_secret_access_key: SiteSetting.s3_secret_access_key + } + options[:region] = SiteSetting.s3_region unless SiteSetting.s3_region.empty? + + options + end + + def self.get_or_create_directory(name, options) + fog = Fog::Storage.new(options) + directory = fog.directories.get(name) + directory = fog.directories.create(key: name) unless directory + + directory + end + + def self.upload(file, name, directory) + directory.files.create( + key: name, + public: true, + body: file.tempfile, + content_type: file.content_type + ) + end + +end diff --git a/spec/components/imgur_spec.rb b/spec/components/imgur_spec.rb index f92717a0514..0b8bada2cd8 100644 --- a/spec/components/imgur_spec.rb +++ b/spec/components/imgur_spec.rb @@ -3,12 +3,18 @@ require 'imgur' describe Imgur do - describe "upload_file" do + describe "store_file" do let(:file) { Rails.root.join('app', 'assets', 'images', 'logo.png') } + let(:image_info) { FastImage.new(file) } let(:params) { [SiteSetting.imgur_endpoint, { image: Base64.encode64(file.read) }, { 'Authorization' => "Client-ID #{SiteSetting.imgur_client_id}" }] } - it 'returns JSON of the Imgur upload if successful' do + before(:each) do + SiteSetting.stubs(:imgur_endpoint).returns("imgur_endpoint") + SiteSetting.stubs(:imgur_client_id).returns("imgur_client_id") + end + + it 'returns the url of the Imgur upload if successful' do json = { data: { id: 'fake', @@ -21,20 +27,9 @@ describe Imgur do response = mock response.expects(:body).returns(json) - - image_info = { - url: 'http://imgur.com/fake.png', - filesize: File.size(file) - } - RestClient.expects(:post).with(*params).returns(response) - result = Imgur.upload_file(file) - # Not testing what width/height actually are because ImageSizer is already tested - result[:url].should eq(image_info[:url]) - result[:filesize].should eq(image_info[:filesize]) - result[:width].should_not be_nil - result[:height].should_not be_nil + Imgur.store_file(file, image_info, 1).should == 'http://imgur.com/fake.png' end it 'returns nil if the request fails' do @@ -47,7 +42,7 @@ describe Imgur do response.expects(:body).returns(json) RestClient.expects(:post).with(*params).returns(response) - Imgur.upload_file(file).should be_nil + Imgur.store_file(file, image_info, 1).should be_nil end end diff --git a/spec/components/local_store_spec.rb b/spec/components/local_store_spec.rb new file mode 100644 index 00000000000..c203670670a --- /dev/null +++ b/spec/components/local_store_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' +require 'local_store' + +describe LocalStore do + + describe "store_file" do + + let(:file) do + ActionDispatch::Http::UploadedFile.new({ + filename: 'logo.png', + content_type: 'image/png', + tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") + }) + end + + let(:image_info) { FastImage.new(file) } + + it 'returns the url of the S3 upload if successful' do + # prevent the tests from creating directories & files... + FileUtils.stubs(:mkdir_p) + File.stubs(:open) + # The Time needs to be frozen as it is used to generate a clean & unique name + Time.stubs(:now).returns(Time.utc(2013, 2, 17, 12, 0, 0, 0)) + # + LocalStore.store_file(file, image_info, 1).should == '/uploads/default/1/253dc8edf9d4ada1.png' + end + + end + +end diff --git a/spec/components/s3_spec.rb b/spec/components/s3_spec.rb new file mode 100644 index 00000000000..009c1ffbe0e --- /dev/null +++ b/spec/components/s3_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' +require 'fog' +require 's3' + +describe S3 do + + describe "store_file" do + + let(:file) do + ActionDispatch::Http::UploadedFile.new({ + filename: 'logo.png', + content_type: 'image/png', + tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") + }) + end + + let(:image_info) { FastImage.new(file) } + + before(:each) do + SiteSetting.stubs(:s3_upload_bucket).returns("s3_upload_bucket") + SiteSetting.stubs(:s3_access_key_id).returns("s3_access_key_id") + SiteSetting.stubs(:s3_secret_access_key).returns("s3_secret_access_key") + Fog.mock! + end + + it 'returns the url of the S3 upload if successful' do + S3.store_file(file, image_info, 1).should == '//s3_upload_bucket.s3.amazonaws.com/1e8b1353813a7d091231f9a27f03566f123463fc1.png' + end + + after(:each) do + Fog.unmock! + end + + end + +end diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 8d214864a19..b91436323aa 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -1,6 +1,4 @@ require 'spec_helper' -require 'fog' -require 'imgur' describe Upload do @@ -23,95 +21,45 @@ describe Upload do }) end - it "uses imgur when it is enabled" do - SiteSetting.stubs(:enable_imgur?).returns(true) - Upload.expects(:create_on_imgur).with(user_id, logo, topic_id) - Upload.create_for(user_id, logo, topic_id) - end + let(:upload) { Upload.create_for(user_id, logo, topic_id) } - it "uses s3 when it is enabled" do - SiteSetting.stubs(:enable_s3_uploads?).returns(true) - Upload.expects(:create_on_s3).with(user_id, logo, topic_id) - Upload.create_for(user_id, logo, topic_id) - end - - it "uses local storage otherwise" do - Upload.expects(:create_locally).with(user_id, logo, topic_id) - Upload.create_for(user_id, logo, topic_id) - end + let(:url) { "http://domain.com" } shared_examples_for "upload" do it "is valid" do + upload.user_id.should == user_id + upload.topic_id.should == topic_id upload.original_filename.should == logo.original_filename - upload.filesize.should == logo.size + upload.filesize.should == File.size(logo.tempfile) upload.width.should == 244 upload.height.should == 66 + upload.url.should == url end end - context 'imgur' do - + context "imgur" do before(:each) do - # Stub out Imgur entirely as it already is tested. - Imgur.stubs(:upload_file).returns({ - url: "imgurlink", - filesize: logo.size, - width: 244, - height: 66 - }) + SiteSetting.stubs(:enable_imgur?).returns(true) + Imgur.stubs(:store_file).returns(url) end - let(:upload) { Upload.create_on_imgur(user_id, logo, topic_id) } - it_behaves_like "upload" - it "works" do - upload.url.should == "imgurlink" - end - end - context 's3' do - + context "s3" do before(:each) do - SiteSetting.stubs(:s3_upload_bucket).returns("bucket") - Fog.mock! + SiteSetting.stubs(:enable_s3_uploads?).returns(true) + S3.stubs(:store_file).returns(url) end - let(:upload) { Upload.create_on_s3(user_id, logo, topic_id) } - it_behaves_like "upload" - it "works" do - upload.url.should == "//bucket.s3-us-west-1.amazonaws.com/e8b1353813a7d091231f9a27f03566f123463fc1.png" - end - - after(:each) do - Fog.unmock! - end - end - context 'local' do - - before(:each) do - # prevent the tests from creating directories & files... - FileUtils.stubs(:mkdir_p) - File.stubs(:open) - end - - let(:upload) do - # The Time needs to be frozen as it is used to generate a clean & unique name - Time.stubs(:now).returns(Time.utc(2013, 2, 17, 12, 0, 0, 0)) - Upload.create_locally(user_id, logo, topic_id) - end - + context "locally" do + before(:each) { LocalStore.stubs(:store_file).returns(url) } it_behaves_like "upload" - - it "works" do - upload.url.should match /\/uploads\/default\/[\d]+\/253dc8edf9d4ada1.png/ - end - end end