Skip to content

Commit

Permalink
Return a 404 error when attempting to access a deleted Whitehall asset
Browse files Browse the repository at this point in the history
Currently, accessing a soft deleted asset returns a 404.  However, accessing a soft deleted Whitehall asset still attempts to proxy to S3, then returns the resulting XML and 404 proxied back from S3.  This commit changes that behaviour so that Whitehall assets are not proxied and a 404 is served from Asset Manager.

Note: the asset media controller uses `find` to get the records, which raises an exception if no records are found.  Whitehall media controller uses `find_by` which does not raise an exception, therefore we have to raise our own exception and rescue as a 404 in the event of no records being returned.
  • Loading branch information
brucebolt committed Aug 15, 2018
1 parent 21f800f commit ff2f946
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 2 deletions.
9 changes: 7 additions & 2 deletions app/controllers/whitehall_media_controller.rb
Expand Up @@ -2,10 +2,15 @@ class WhitehallMediaController < MediaController

protected

class WhitehallAssetNotFound < StandardError
end

rescue_from WhitehallAssetNotFound, with: :error_404

def asset
@asset ||= WhitehallAsset.from_params(
@asset ||= WhitehallAsset.undeleted.from_params(
path: params[:path], format: params[:format], path_prefix: 'government/uploads/'
)
) || raise(WhitehallAssetNotFound)
end

def asset_servable?
Expand Down
15 changes: 15 additions & 0 deletions spec/controllers/whitehall_media_controller_spec.rb
Expand Up @@ -253,5 +253,20 @@
expect(response).to have_http_status(:not_found)
end
end

context 'with a soft deleted file' do
let(:state) { 'uploaded' }

before do
allow(WhitehallAsset).to receive(:find_by).with(legacy_url_path: legacy_url_path).and_return(nil)
asset.update_attribute(:deleted_at, Time.now)
end

it 'responds with not found status' do
get :download, params: { path: path, format: format }

expect(response).to have_http_status(:not_found)
end
end
end
end
27 changes: 27 additions & 0 deletions spec/models/whitehall_asset_spec.rb
Expand Up @@ -176,4 +176,31 @@
end
end
end

describe 'soft deletion' do
let(:asset) { FactoryBot.create(:whitehall_asset) }

before do
asset.destroy
end

it 'adds a deleted_at timestamp to the record' do
expect(asset.deleted_at).not_to be_nil
end

it 'is not inclued in the "undeleted" scope' do
expect(Asset.undeleted).not_to include(asset)
end

it 'is included in the "deleted" scope' do
expect(Asset.deleted).to include(asset)
end

it 'can be restored' do
asset.destroy
expect(asset.deleted_at).not_to be_nil
asset.restore
expect(asset.deleted_at).to be_nil
end
end
end

0 comments on commit ff2f946

Please sign in to comment.