Skip to content

Commit

Permalink
Upload assets to S3 with Cache-Control header set
Browse files Browse the repository at this point in the history
We want to be able to redirect Asset Manager requests to the assets
stored in an S3 bucket. However, we want as much of the
externally-visible behaviour to remain the same and so we need to set
the 'Cache-Control' header to the same value as in
`MediaController#download`, i.e. the value generated by
`ActionController::ConditionalGet#expires_in` [1] using
`ApplicationController#set_expiry`.

Note that the header is set on the S3 object at the point where it is
uploaded. See the documentation for `Aws::S3::Object#upload_file` [2].

[1]: http://api.rubyonrails.org/v4.2.7.1/classes/ActionController/ConditionalGet.html#method-i-expires_in
[2]: http://docs.aws.amazon.com/sdkforruby/api/Aws/S3/Object.html#upload_file-instance_method
  • Loading branch information
floehopper committed Aug 3, 2017
1 parent 278f180 commit 1122e48
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 5 deletions.
2 changes: 1 addition & 1 deletion app/models/asset.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def accessible_by?(user)
end

def save_to_cloud_storage
Services.cloud_storage.save(self)
Services.cloud_storage.save(self, cache_control: AssetManager.cache_control.header)
rescue => e
Airbrake.notify_or_ignore(e, params: { id: self.id, filename: self.filename })
raise
Expand Down
9 changes: 9 additions & 0 deletions lib/cache_control_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,13 @@ def max_age
def options
@attributes.except(:max_age)
end

def header
response = ActionDispatch::Response.new
@attributes.each do |key, value|
response.cache_control[key] = value
end
response.prepare!
response['Cache-Control']
end
end
4 changes: 2 additions & 2 deletions lib/s3_storage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ def initialize(bucket_name)
@bucket_name = bucket_name
end

def save(asset)
object_for(asset).upload_file(asset.file.path)
def save(asset, options = {})
object_for(asset).upload_file(asset.file.path, options)
end

def load(asset)
Expand Down
18 changes: 18 additions & 0 deletions spec/lib/cache_control_configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,22 @@
expect(subject.options).to eq(public: true)
end
end

describe '#header' do
context 'when attributes are supplied to constructor' do
let(:attributes) { { max_age: 1.hour, public: true, must_revalidate: true } }

it 'returns Cache-Control header value c.f. expires_in' do
expect(subject.header).to eq('max-age=3600, public, must-revalidate')
end
end

context 'when only max_age attribute is supplied to constructor' do
let(:attributes) { { max_age: 1.hour } }

it "returns Cache-Control header value with default visibility" do
expect(subject.header).to include('private')
end
end
end
end
8 changes: 7 additions & 1 deletion spec/lib/s3_storage_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,17 @@

describe '#save' do
it 'uploads file to S3 bucket' do
expect(s3_object).to receive(:upload_file).with(asset.file.path)
expect(s3_object).to receive(:upload_file).with(asset.file.path, anything)

subject.save(asset)
end

it 'passes options to Aws::S3::Object#upload_file' do
expect(s3_object).to receive(:upload_file).with(anything, cache_control: 'cache-control-header')

subject.save(asset, cache_control: 'cache-control-header')
end

context 'when bucket name is blank' do
let(:bucket_name) { '' }

Expand Down
11 changes: 10 additions & 1 deletion spec/models/asset_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,22 @@
describe "#save_to_cloud_storage" do
let(:asset) { FactoryGirl.create(:clean_asset) }
let(:cloud_storage) { double(:cloud_storage) }
let(:cache_control) { instance_double(CacheControlConfiguration) }

before do
allow(Services).to receive(:cloud_storage).and_return(cloud_storage)
allow(AssetManager).to receive(:cache_control).and_return(cache_control)
allow(cache_control).to receive(:header).and_return('cache-control-header')
end

it 'saves the asset to cloud storage' do
expect(cloud_storage).to receive(:save).with(asset)
expect(cloud_storage).to receive(:save).with(asset, anything)

asset.save_to_cloud_storage
end

it 'sets the Cache-Control header on the asset stored in the cloud' do
expect(cloud_storage).to receive(:save).with(anything, cache_control: 'cache-control-header')

asset.save_to_cloud_storage
end
Expand Down

0 comments on commit 1122e48

Please sign in to comment.