diff --git a/app/services/collection_resources_service.rb b/app/services/collection_resources_service.rb index 42667bb78..5116c788b 100644 --- a/app/services/collection_resources_service.rb +++ b/app/services/collection_resources_service.rb @@ -20,13 +20,7 @@ class CollectionResourcesService end def file_exists_on_s3?(file) - url = "https://#{@storage_service.configuration.bucket_name}.s3.amazonaws.com/#{file}" - uri = URI.parse(url) - - response = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https") do |http| - http.request_head(uri) - end - response.is_a?(Net::HTTPSuccess) || response.is_a?(Net::HTTPRedirection) + @storage_service.file_exists?(file) end def upload_collection_resource(filename, file) diff --git a/app/services/storage/s3_service.rb b/app/services/storage/s3_service.rb index 0cd4a1f20..e6776df0b 100644 --- a/app/services/storage/s3_service.rb +++ b/app/services/storage/s3_service.rb @@ -43,6 +43,13 @@ module Storage @client.head_object(bucket: @configuration.bucket_name, key: file_name) end + def file_exists?(file_name) + @client.head_object(bucket: @configuration.bucket_name, key: file_name) + true + rescue Aws::S3::Errors::NotFound + false + end + private def create_configuration diff --git a/spec/features/collection_resources_spec.rb b/spec/features/collection_resources_spec.rb index aaf4b30d4..643f3537e 100644 --- a/spec/features/collection_resources_spec.rb +++ b/spec/features/collection_resources_spec.rb @@ -7,7 +7,7 @@ RSpec.describe "Collection resources" do before do allow(CollectionResourcesService).to receive(:new).and_return(collection_resources_service) allow(collection_resources_service).to receive(:upload_collection_resource) - allow(collection_resources_service).to receive(:get_file_metadata).and_return({ "Content-Type" => "application/pdf", "Content-Length" => 1000 }) + allow(collection_resources_service).to receive(:get_file_metadata).and_return({ "content_type" => "application/pdf", "content_length" => 1000 }) allow(user).to receive(:need_two_factor_authentication?).and_return(false) sign_in user end diff --git a/spec/helpers/collection_resources_helper_spec.rb b/spec/helpers/collection_resources_helper_spec.rb index e50dd334b..31033d997 100644 --- a/spec/helpers/collection_resources_helper_spec.rb +++ b/spec/helpers/collection_resources_helper_spec.rb @@ -115,10 +115,8 @@ RSpec.describe CollectionResourcesHelper do end before do - WebMock.stub_request(:head, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com\/2023_24_lettings_paper_form.pdf/) - .to_return(status: 200, body: "", headers: { "Content-Length" => 292_864, "Content-Type" => "application/pdf" }) - WebMock.stub_request(:head, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com\/2023_24_lettings_bulk_upload_template.xlsx/) - .to_return(status: 200, body: "", headers: { "Content-Length" => 19_456, "Content-Type" => "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" }) + allow(storage_service).to receive(:get_file_metadata).with("2023_24_lettings_paper_form.pdf").and_return("content_length" => 292_864, "content_type" => "application/pdf") + allow(storage_service).to receive(:get_file_metadata).with("2023_24_lettings_bulk_upload_template.xlsx").and_return("content_length" => 19_456, "content_type" => "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet") end it "returns component items" do @@ -146,10 +144,8 @@ RSpec.describe CollectionResourcesHelper do end before do - WebMock.stub_request(:head, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com\/2023_24_lettings_paper_form.pdf/) - .to_return(status: 200, body: "", headers: { "Content-Length" => 292_864, "Content-Type" => "application/pdf" }) - WebMock.stub_request(:head, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com\/2023_24_lettings_bulk_upload_template.xlsx/) - .to_return(status: 200, body: "", headers: { "Content-Length" => 19_456, "Content-Type" => "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" }) + allow(storage_service).to receive(:get_file_metadata).with("2023_24_lettings_paper_form.pdf").and_return("content_length" => 292_864, "content_type" => "application/pdf") + allow(storage_service).to receive(:get_file_metadata).with("2023_24_lettings_bulk_upload_template.xlsx").and_return("content_length" => 19_456, "content_type" => "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet") end it "returns component items" do diff --git a/spec/requests/collection_resources_controller_spec.rb b/spec/requests/collection_resources_controller_spec.rb index fb8b7ab6f..d7735746c 100644 --- a/spec/requests/collection_resources_controller_spec.rb +++ b/spec/requests/collection_resources_controller_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" RSpec.describe CollectionResourcesController, type: :request do let(:page) { Capybara::Node::Simple.new(response.body) } - let(:storage_service) { instance_double(Storage::S3Service) } + let(:storage_service) { instance_double(Storage::S3Service, get_file_metadata: nil) } before do allow(Storage::S3Service).to receive(:new).and_return(storage_service) @@ -49,6 +49,7 @@ RSpec.describe CollectionResourcesController, type: :request do before do allow(Time.zone).to receive(:today).and_return(Time.zone.local(2025, 1, 8)) allow(user).to receive(:need_two_factor_authentication?).and_return(false) + allow(storage_service).to receive(:file_exists?).and_return(true) sign_in user end @@ -71,6 +72,7 @@ RSpec.describe CollectionResourcesController, type: :request do context "when files are on S3" do before do + allow(storage_service).to receive(:file_exists?).and_return(true) get collection_resources_path end @@ -110,8 +112,7 @@ RSpec.describe CollectionResourcesController, type: :request do context "when files are not on S3" do before do - WebMock.stub_request(:head, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com/) - .to_return(status: 404, body: "", headers: {}) + allow(storage_service).to receive(:file_exists?).and_return(false) get collection_resources_path end @@ -141,8 +142,7 @@ RSpec.describe CollectionResourcesController, type: :request do context "when the file exists on S3" do before do - WebMock.stub_request(:get, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com/) - .to_return(status: 200, body: "file", headers: { "Content-Type" => "application/pdf", "Content-Length" => 1000 }) + allow(storage_service).to receive(:get_file_io).and_return("file") get download_mandatory_collection_resource_path(log_type: "lettings", year: 2025, resource_type: "paper_form") end @@ -153,8 +153,7 @@ RSpec.describe CollectionResourcesController, type: :request do context "when the file does not exist on S3" do before do - WebMock.stub_request(:get, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com/) - .to_return(status: 404, body: "", headers: {}) + allow(storage_service).to receive(:get_file_io).and_return(nil) get download_mandatory_collection_resource_path(log_type: "lettings", year: 2024, resource_type: "paper_form") end @@ -189,8 +188,7 @@ RSpec.describe CollectionResourcesController, type: :request do context "when year is in editable_collection_resource_years but not in displayed_collection_resource_years" do before do - WebMock.stub_request(:get, /https:\/\/core-test-collection-resources\.s3\.amazonaws\.com/) - .to_return(status: 200, body: "file", headers: { "Content-Type" => "application/pdf", "Content-Length" => 1000 }) + allow(storage_service).to receive(:get_file_io).and_return("file") get download_mandatory_collection_resource_path(log_type: "lettings", year: 2026, resource_type: "paper_form") end @@ -262,9 +260,10 @@ RSpec.describe CollectionResourcesController, type: :request do describe "PATCH #update_mandatory_collection_resource" do let(:some_file) { File.open(file_fixture("blank_bulk_upload_sales.csv")) } let(:params) { { collection_resource: { year: 2024, log_type: "sales", resource_type: "bulk_upload_template", file: some_file } } } + let(:collection_resource_service) { instance_double(CollectionResourcesService) } before do - allow(UploadCollectionResourcesService).to receive(:upload_collection_resource) + allow(CollectionResourcesService).to receive(:new).and_return(collection_resource_service) end context "when user is not signed in" do