diff --git a/app/models/upload.rb b/app/models/upload.rb index 02c3d0c2fba..91aea72e424 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -17,7 +17,7 @@ class Upload < ActiveRecord::Base validates_presence_of :original_filename def thumbnail - @thumbnail ||= optimized_images.where(width: width, height: height).first + optimized_images.where(width: width, height: height).first end def thumbnail_url @@ -48,23 +48,25 @@ class Upload < ActiveRecord::Base sha1 = Digest::SHA1.file(file.tempfile).hexdigest # check if the file has already been uploaded unless upload = Upload.where(sha1: sha1).first + # deal with width & heights for images + if SiteSetting.authorized_image?(file) + # retrieve image info + image_info = FastImage.new(file.tempfile, raise_on_failure: true) + # compute image aspect ratio + width, height = ImageSizer.resize(*image_info.size) + # make sure we're at the beginning of the file (FastImage is moving the pointer) + file.rewind + end # create a db record (so we can use the id) upload = Upload.create!({ user_id: user_id, original_filename: file.original_filename, filesize: File.size(file.tempfile), sha1: sha1, - url: "" + url: "", + width: width, + height: height, }) - # deal with width & heights for images - if SiteSetting.authorized_image?(file) - # retrieve image info - image_info = FastImage.new(file.tempfile, raise_on_failure: true) - # compute image aspect ratio - upload.width, upload.height = ImageSizer.resize(*image_info.size) - # make sure we're at the beginning of the file (FastImage is moving the pointer) - file.rewind - end # store the file and update its url upload.url = Upload.store_file(file, sha1, upload.id) # save the url @@ -80,8 +82,8 @@ class Upload < ActiveRecord::Base end def self.remove_file(url) - S3.remove_file(url) if SiteSetting.enable_s3_uploads? - LocalStore.remove_file(url) + return S3.remove_file(url) if SiteSetting.enable_s3_uploads? + return LocalStore.remove_file(url) end def self.has_been_uploaded?(url) diff --git a/spec/fabricators/optimized_image_fabricator.rb b/spec/fabricators/optimized_image_fabricator.rb new file mode 100644 index 00000000000..3ca2fe71f9a --- /dev/null +++ b/spec/fabricators/optimized_image_fabricator.rb @@ -0,0 +1,7 @@ +Fabricator(:optimized_image) do + upload + sha1 "abcdef" + extension ".png" + width 100 + height 200 +end diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 850fb841034..139e484db86 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -13,52 +13,142 @@ describe Upload do it { should validate_presence_of :original_filename } it { should validate_presence_of :filesize } - context '.create_for' do + let(:upload) { build(:upload) } + let(:thumbnail) { build(:optimized_image, upload: upload) } - let(:user_id) { 1 } + let(:user_id) { 1 } + let(:url) { "http://domain.com" } - let(:logo) do - ActionDispatch::Http::UploadedFile.new({ - filename: 'logo.png', - content_type: 'image/png', - tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") - }) + let(:image) do + ActionDispatch::Http::UploadedFile.new({ + filename: 'logo.png', + tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") + }) + end + + let(:image_sha1) { Digest::SHA1.file(image.tempfile).hexdigest } + + let(:attachment) do + ActionDispatch::Http::UploadedFile.new({ + filename: File.basename(__FILE__), + tempfile: File.new(__FILE__) + }) + end + + context ".create_thumbnail!" do + + it "does not create a thumbnail when disabled" do + SiteSetting.stubs(:create_thumbnails?).returns(false) + SiteSetting.expects(:enable_s3_uploads?).never + upload.create_thumbnail! end - let(:upload) { Upload.create_for(user_id, logo) } - - let(:url) { "http://domain.com" } - - shared_examples_for "upload" do - it "is valid" do - upload.user_id.should == user_id - upload.original_filename.should == logo.original_filename - upload.filesize.should == File.size(logo.tempfile) - upload.sha1.should == Digest::SHA1.file(logo.tempfile).hexdigest - upload.width.should == 244 - upload.height.should == 66 - upload.url.should == url - end + it "does not create a thumbnail when using S3" do + SiteSetting.expects(:create_thumbnails?).returns(true) + SiteSetting.expects(:enable_s3_uploads?).returns(true) + upload.expects(:has_thumbnail?).never + upload.create_thumbnail! end - context "s3" do - before(:each) do - SiteSetting.stubs(:enable_s3_uploads?).returns(true) - S3.stubs(:store_file).returns(url) - end - - it_behaves_like "upload" - + it "does not create another thumbnail" do + SiteSetting.expects(:create_thumbnails?).returns(true) + SiteSetting.expects(:enable_s3_uploads?).returns(false) + upload.expects(:has_thumbnail?).returns(true) + OptimizedImage.expects(:create_for).never + upload.create_thumbnail! end - context "locally" do - before(:each) { LocalStore.stubs(:store_file).returns(url) } - it_behaves_like "upload" + it "creates a thumbnail" do + upload = Fabricate(:upload) + thumbnail = Fabricate(:optimized_image, upload: upload) + SiteSetting.expects(:create_thumbnails?).returns(true) + SiteSetting.expects(:enable_s3_uploads?).returns(false) + upload.expects(:has_thumbnail?).returns(false) + OptimizedImage.expects(:create_for).returns(thumbnail) + upload.create_thumbnail! + upload.reload + upload.optimized_images.count.should == 1 end end - context 'has_been_uploaded?' do + context ".create_for" do + + it "does not create another upload if it already exists" do + Upload.expects(:where).with(sha1: image_sha1).returns([upload]) + Upload.expects(:create!).never + Upload.create_for(user_id, image).should == upload + end + + it "computes width & height for images" do + SiteSetting.expects(:authorized_image?).returns(true) + FastImage.any_instance.expects(:size).returns([100, 200]) + ImageSizer.expects(:resize) + ActionDispatch::Http::UploadedFile.any_instance.expects(:rewind) + Upload.create_for(user_id, image) + end + + it "does not create an upload when there is an error with FastImage" do + SiteSetting.expects(:authorized_image?).returns(true) + Upload.expects(:create!).never + expect { Upload.create_for(user_id, attachment) }.to raise_error(FastImage::UnknownImageType) + end + + it "does not compute width & height for non-image" do + SiteSetting.expects(:authorized_image?).returns(false) + FastImage.any_instance.expects(:size).never + Upload.create_for(user_id, image) + end + + it "saves proper information" do + Upload.expects(:store_file).returns(url) + upload = Upload.create_for(user_id, image) + upload.user_id.should == user_id + upload.original_filename.should == image.original_filename + upload.filesize.should == File.size(image.tempfile) + upload.sha1.should == Digest::SHA1.file(image.tempfile).hexdigest + upload.width.should == 244 + upload.height.should == 66 + upload.url.should == url + end + + end + + context ".store_file" do + + it "store files on s3 when enabled" do + SiteSetting.expects(:enable_s3_uploads?).returns(true) + LocalStore.expects(:store_file).never + S3.expects(:store_file) + Upload.store_file(image, image_sha1, 1) + end + + it "store files locally by default" do + S3.expects(:store_file).never + LocalStore.expects(:store_file) + Upload.store_file(image, image_sha1, 1) + end + + end + + context ".remove_file" do + + it "remove files on s3 when enabled" do + SiteSetting.expects(:enable_s3_uploads?).returns(true) + LocalStore.expects(:remove_file).never + S3.expects(:remove_file) + Upload.remove_file(upload.url) + end + + it "remove files locally by default" do + S3.expects(:remove_file).never + LocalStore.expects(:remove_file) + Upload.remove_file(upload.url) + end + + end + + context ".has_been_uploaded?" do it "identifies internal or relatives urls" do Discourse.expects(:base_url_no_prefix).returns("http://discuss.site.com") @@ -84,4 +174,14 @@ describe Upload do end + context ".get_from_url" do + + it "works only when the file has been uploaded" do + Upload.expects(:has_been_uploaded?).returns(false) + Upload.expects(:where).never + Upload.get_from_url("discourse.org") + end + + end + end