diff --git a/lib/carrierwave/storage/fog.rb b/lib/carrierwave/storage/fog.rb index ceb33aa46..074409a8e 100644 --- a/lib/carrierwave/storage/fog.rb +++ b/lib/carrierwave/storage/fog.rb @@ -276,6 +276,16 @@ def initialize(uploader, base, path) # # [String] contents of file def read + file_body = file.body + + return if file_body.nil? + return file_body unless file_body.is_a?(::File) + + # Fog::Storage::XXX::File#body could return the source file which was upoloaded to the remote server. + read_source_file(file_body) if ::File.exist?(file_body.path) + + # If the source file doesn't exist, the remote content is read + @file = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables file.body end @@ -313,7 +323,7 @@ def store(new_file) fog_file = new_file.to_file @content_type ||= new_file.content_type @file = directory.files.create({ - :body => (fog_file ? fog_file : new_file).read, + :body => fog_file ? fog_file : new_file.read, :content_type => @content_type, :key => path, :public => @uploader.fog_public @@ -458,6 +468,17 @@ def file def acl_header {'x-amz-acl' => @uploader.fog_public ? 'public-read' : 'private'} end + + def read_source_file(file_body) + return unless ::File.exist?(file_body.path) + + begin + file_body = ::File.open(file_body.path) if file_body.closed? # Reopen if it's already closed + file_body.read + ensure + file_body.close + end + end end end # Fog diff --git a/spec/storage/fog_helper.rb b/spec/storage/fog_helper.rb index 8aab7c926..1e89bbbf7 100644 --- a/spec/storage/fog_helper.rb +++ b/spec/storage/fog_helper.rb @@ -45,174 +45,197 @@ class FogSpec#{fog_credentials[:provider]}Uploader < CarrierWave::Uploader::Base end describe '#store!' do - let(:store_path) { 'uploads/test+.jpg' } - before do - allow(@uploader).to receive(:store_path).and_return(store_path) - @fog_file = @storage.store!(file) - end + context 'when file is ::File' do + before do + allow(@uploader).to receive(:store_path).and_return(store_path) + @fog_file = @storage.store!(file) + end - it "should upload the file" do - # reading the file after upload should return the body, not a closed tempfile - expect(@fog_file.read).to eq('this is stuff') - # make sure it actually uploaded to the service, too - expect(@directory.files.get(store_path).body).to eq('this is stuff') - end + it "should upload the file" do + # reading the file after upload should return the body, not a closed tempfile + expect(@fog_file.read).to eq('this is stuff') + # make sure it actually uploaded to the service, too + expect(@directory.files.get(store_path).body).to eq('this is stuff') + end - it "should have a path" do - expect(@fog_file.path).to eq(store_path) - end + it "should have a path" do + expect(@fog_file.path).to eq(store_path) + end - it "should have a content_type" do - expect(@fog_file.content_type).to eq(file.content_type) - expect(@directory.files.get(store_path).content_type).to eq(file.content_type) - end + it "should have a content_type" do + expect(@fog_file.content_type).to eq(file.content_type) + expect(@directory.files.get(store_path).content_type).to eq(file.content_type) + end - it "should have an extension" do - expect(@fog_file.extension).to eq("jpg") - end + it "should have an extension" do + expect(@fog_file.extension).to eq("jpg") + end - context "without asset_host" do - it "should have a public_url" do - unless fog_credentials[:provider] == 'Local' - expect(@fog_file.public_url).not_to be_nil + context "without asset_host" do + it "should have a public_url" do + unless fog_credentials[:provider] == 'Local' + expect(@fog_file.public_url).not_to be_nil + end end - end - it "should have a url" do - unless fog_credentials[:provider] == 'Local' - expect(@fog_file.url).not_to be_nil + it "should have a url" do + unless fog_credentials[:provider] == 'Local' + expect(@fog_file.url).not_to be_nil + end end - end - context "directory is a valid subdomain" do - before do - allow(@uploader).to receive(:fog_directory).and_return('assets.site.com') + context "directory is a valid subdomain" do + before do + allow(@uploader).to receive(:fog_directory).and_return('assets.site.com') + end + + it "should use a subdomain URL for AWS" do + if @provider == 'AWS' + expect(@fog_file.public_url).to include('https://assets.site.com.s3.amazonaws.com') + end + end + + it "should use accelerate domain if fog_aws_accelerate is true" do + if @provider == 'AWS' + allow(@uploader).to receive(:fog_aws_accelerate).and_return(true) + expect(@fog_file.public_url).to include('https://assets.site.com.s3-accelerate.amazonaws.com') + end + end end - it "should use a subdomain URL for AWS" do + it "should not use a subdomain URL for AWS if the directory is not a valid subdomain" do if @provider == 'AWS' - expect(@fog_file.public_url).to include('https://assets.site.com.s3.amazonaws.com') + allow(@uploader).to receive(:fog_directory).and_return('SiteAssets') + expect(@fog_file.public_url).to include('https://s3.amazonaws.com/SiteAssets') end end - it "should use accelerate domain if fog_aws_accelerate is true" do + it "should use https as a default protocol" do if @provider == 'AWS' - allow(@uploader).to receive(:fog_aws_accelerate).and_return(true) - expect(@fog_file.public_url).to include('https://assets.site.com.s3-accelerate.amazonaws.com') + expect(@fog_file.public_url).to start_with 'https://' end end - end - it "should not use a subdomain URL for AWS if the directory is not a valid subdomain" do - if @provider == 'AWS' - allow(@uploader).to receive(:fog_directory).and_return('SiteAssets') - expect(@fog_file.public_url).to include('https://s3.amazonaws.com/SiteAssets') + it "should use https as a default protocol" do + if @provider == 'AWS' + allow(@uploader).to receive(:fog_use_ssl_for_aws).and_return(false) + expect(@fog_file.public_url).to start_with 'http://' + end end - end - it "should use https as a default protocol" do - if @provider == 'AWS' - expect(@fog_file.public_url).to start_with 'https://' + it "should use the google public url if available" do + if @provider == 'Google' + allow(@uploader).to receive(:fog_directory).and_return('SiteAssets') + expect(@fog_file.public_url).to include('https://storage.googleapis.com/SiteAssets') + end end end - it "should use https as a default protocol" do - if @provider == 'AWS' - allow(@uploader).to receive(:fog_use_ssl_for_aws).and_return(false) - expect(@fog_file.public_url).to start_with 'http://' - end - end + context "with asset_host" do + before { allow(@uploader).to receive(:asset_host).and_return(asset_host) } - it "should use the google public url if available" do - if @provider == 'Google' - allow(@uploader).to receive(:fog_directory).and_return('SiteAssets') - expect(@fog_file.public_url).to include('https://storage.googleapis.com/SiteAssets') - end - end - end + context "when a asset_host is a proc" do - context "with asset_host" do - before { allow(@uploader).to receive(:asset_host).and_return(asset_host) } + let(:asset_host) { proc { "http://foo.bar" } } - context "when a asset_host is a proc" do + describe "args passed to proc" do + let(:asset_host) { proc { |storage| expect(storage).to be_instance_of ::CarrierWave::Storage::Fog::File } } - let(:asset_host) { proc { "http://foo.bar" } } + it "should be the uploader" do + @fog_file.public_url + end + end - describe "args passed to proc" do - let(:asset_host) { proc { |storage| expect(storage).to be_instance_of ::CarrierWave::Storage::Fog::File } } + it "should have a asset_host rooted public_url" do + expect(@fog_file.public_url).to eq('http://foo.bar/uploads/test%2B.jpg') + end - it "should be the uploader" do - @fog_file.public_url + it "should have a asset_host rooted url" do + expect(@fog_file.url).to eq('http://foo.bar/uploads/test%2B.jpg') end - end - it "should have a asset_host rooted public_url" do - expect(@fog_file.public_url).to eq('http://foo.bar/uploads/test%2B.jpg') - end + it "should always have the same asset_host rooted url" do + expect(@fog_file.url).to eq('http://foo.bar/uploads/test%2B.jpg') + expect(@fog_file.url).to eq('http://foo.bar/uploads/test%2B.jpg') + end - it "should have a asset_host rooted url" do - expect(@fog_file.url).to eq('http://foo.bar/uploads/test%2B.jpg') + it 'should retrieve file name' do + expect(@fog_file.filename).to eq('test+.jpg') + end end - it "should always have the same asset_host rooted url" do - expect(@fog_file.url).to eq('http://foo.bar/uploads/test%2B.jpg') - expect(@fog_file.url).to eq('http://foo.bar/uploads/test%2B.jpg') - end + context "when a string" do + let(:asset_host) { "http://foo.bar" } + + it "should have a asset_host rooted public_url" do + expect(@fog_file.public_url).to eq('http://foo.bar/uploads/test%2B.jpg') + end + + it "should have a asset_host rooted url" do + expect(@fog_file.url).to eq('http://foo.bar/uploads/test%2B.jpg') + end - it 'should retrieve file name' do - expect(@fog_file.filename).to eq('test+.jpg') + it "should always have the same asset_host rooted url" do + expect(@fog_file.url).to eq('http://foo.bar/uploads/test%2B.jpg') + expect(@fog_file.url).to eq('http://foo.bar/uploads/test%2B.jpg') + end end + end - context "when a string" do - let(:asset_host) { "http://foo.bar" } + context "without extension" do - it "should have a asset_host rooted public_url" do - expect(@fog_file.public_url).to eq('http://foo.bar/uploads/test%2B.jpg') - end + let(:store_path) { 'uploads/test' } - it "should have a asset_host rooted url" do - expect(@fog_file.url).to eq('http://foo.bar/uploads/test%2B.jpg') + it "should have no extension" do + expect(@fog_file.extension).to be_nil end - it "should always have the same asset_host rooted url" do - expect(@fog_file.url).to eq('http://foo.bar/uploads/test%2B.jpg') - expect(@fog_file.url).to eq('http://foo.bar/uploads/test%2B.jpg') - end end - end - - context "without extension" do - - let(:store_path) { 'uploads/test' } + it "should return filesize" do + expect(@fog_file.size).to eq(13) + end - it "should have no extension" do - expect(@fog_file.extension).to be_nil + it "should be deletable" do + @fog_file.delete + expect(@directory.files.head(store_path)).to eq(nil) end - end + context "when the file has been deleted" do + before { @fog_file.delete } - it "should return filesize" do - expect(@fog_file.size).to eq(13) - end + it "should not error getting the file size" do + expect { @fog_file.size }.not_to raise_error + end - it "should be deletable" do - @fog_file.delete - expect(@directory.files.head(store_path)).to eq(nil) + it "should not error getting the content type" do + expect { @fog_file.content_type }.not_to raise_error + end + end end - context "when the file has been deleted" do - before { @fog_file.delete } + context 'when file is ::StringIO' do + let(:file) do + CarrierWave::SanitizedFile.new( + :tempfile => StringIO.new('Test StringIO texts'), + :filename => 'test.jpg', + :content_type => 'image/jpeg' + ) + end - it "should not error getting the file size" do - expect { @fog_file.size }.not_to raise_error + before do + allow(@uploader).to receive(:store_path).and_return(store_path) + @fog_file = @storage.store!(file) end - it "should not error getting the content type" do - expect { @fog_file.content_type }.not_to raise_error + it "should upload the file" do + # reading the file after upload should return the body, not a closed tempfile + expect(@fog_file.read).to eq('Test StringIO texts') + # make sure it actually uploaded to the service, too + expect(@directory.files.get(store_path).body).to eq('Test StringIO texts') end end end