From 8a98310cf95d7d59e18856ef0360b48360e5c6c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Sat, 15 Jun 2013 11:52:40 +0200 Subject: [PATCH] make sure we only do the work once --- app/models/upload.rb | 26 ++++++++++++-------------- lib/local_store.rb | 4 ++-- lib/s3.rb | 6 ++---- spec/components/local_store_spec.rb | 4 ++-- spec/components/s3_spec.rb | 4 ++-- spec/models/upload_spec.rb | 2 ++ 6 files changed, 22 insertions(+), 24 deletions(-) diff --git a/app/models/upload.rb b/app/models/upload.rb index 0cea30b47ec..41c6f536a3d 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -13,17 +13,18 @@ class Upload < ActiveRecord::Base validates_presence_of :original_filename def self.create_for(user_id, 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) # compute the sha sha = Digest::SHA1.file(file.tempfile).hexdigest # check if the file has already been uploaded upload = Upload.where(sha: sha).first + # otherwise, create it if upload.blank? - + # retrieve image info + image_info = FastImage.new(file.tempfile, raise_on_failure: true) + # compute image aspect ratio + width, height = ImageSizer.resize(*image_info.size) + # create a db record (so we can use the id) upload = Upload.create!({ user_id: user_id, original_filename: file.original_filename, @@ -33,23 +34,20 @@ class Upload < ActiveRecord::Base height: height, url: "" }) - # make sure we're at the beginning of the file (FastImage is moving the pointer) file.rewind - # store the file and update its url - upload.url = Upload.store_file(file, image_info, upload.id) - + upload.url = Upload.store_file(file, sha, image_info, upload.id) + # save the url upload.save - end - + # return the uploaded file upload end - def self.store_file(file, image_info, upload_id) - return S3.store_file(file, image_info, upload_id) if SiteSetting.enable_s3_uploads? - return LocalStore.store_file(file, image_info, upload_id) + def self.store_file(file, sha, image_info, upload_id) + return S3.store_file(file, sha, image_info, upload_id) if SiteSetting.enable_s3_uploads? + return LocalStore.store_file(file, sha, image_info, upload_id) end end diff --git a/lib/local_store.rb b/lib/local_store.rb index b08d3b71fc4..928d38dc02f 100644 --- a/lib/local_store.rb +++ b/lib/local_store.rb @@ -1,6 +1,6 @@ module LocalStore - def self.store_file(file, image_info, upload_id) + def self.store_file(file, sha, 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}" @@ -15,4 +15,4 @@ module LocalStore return Discourse::base_uri + "#{url_root}/#{clean_name}" end -end \ No newline at end of file +end diff --git a/lib/s3.rb b/lib/s3.rb index c999a54b297..5df38f0de86 100644 --- a/lib/s3.rb +++ b/lib/s3.rb @@ -1,15 +1,13 @@ module S3 - def self.store_file(file, image_info, upload_id) + def self.store_file(file, sha, 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}" + remote_filename = "#{upload_id}#{sha}.#{image_info.type}" options = S3.generate_options directory = S3.get_or_create_directory(SiteSetting.s3_upload_bucket, options) diff --git a/spec/components/local_store_spec.rb b/spec/components/local_store_spec.rb index c203670670a..7421969d69a 100644 --- a/spec/components/local_store_spec.rb +++ b/spec/components/local_store_spec.rb @@ -21,8 +21,8 @@ describe LocalStore do 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' + # + LocalStore.store_file(file, "", image_info, 1).should == '/uploads/default/1/253dc8edf9d4ada1.png' end end diff --git a/spec/components/s3_spec.rb b/spec/components/s3_spec.rb index 009c1ffbe0e..f2b4d268277 100644 --- a/spec/components/s3_spec.rb +++ b/spec/components/s3_spec.rb @@ -16,7 +16,7 @@ describe S3 do let(:image_info) { FastImage.new(file) } - before(:each) do + 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") @@ -24,7 +24,7 @@ describe S3 do 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' + S3.store_file(file, "SHA", image_info, 1).should == '//s3_upload_bucket.s3.amazonaws.com/1SHA.png' end after(:each) do diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index f44c1f67910..870a1e4ec62 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' +require 'digest/sha1' describe Upload do @@ -31,6 +32,7 @@ describe Upload do upload.user_id.should == user_id upload.original_filename.should == logo.original_filename upload.filesize.should == File.size(logo.tempfile) + upload.sha.should == Digest::SHA1.file(logo.tempfile).hexdigest upload.width.should == 244 upload.height.should == 66 upload.url.should == url