Add validation from wrong period type for the exchange rates + other improv.

app/controllers/api/v2/exchange_rates/files_controller.rb

@@ -3,7 +3,9 @@ module V2
     module ExchangeRates
       class FilesController < ApiController
         def show
-          filename = ExchangeRateFile.filename_for_download(type, format, year, month)
+          validate_params!
+
+          filename = ExchangeRateFile.filename_for_download(period_type, format, year, month)
 
           send_data(
             file_data,
@@ -14,20 +16,22 @@ def show
 
         private
 
-        def type
-          match_data = id.match(/^(monthly_csv_hmrc|monthly_csv|monthly_xml|average_csv|spot_csv)_/)
+        def period_type
+          regexp = Regexp.new("^(#{ExchangeRateFile::APPLICABLE_TYPES.join('|')})_")
+          match_data = param_id.match(regexp)
+
           match_data[1] if match_data
         end
 
         def month
-          @month ||= params[:id].split('-').last
+          @month ||= param_id.split('-').last
         end
 
         def year
-          @year ||= params[:id].split('-').first.split('_').last
+          @year ||= param_id.split('-').first.split('_').last
         end
 
-        def id
+        def param_id
           params[:id].to_s
         end
 
@@ -48,12 +52,19 @@ def file_data
 
         def file
           @file ||= ExchangeRateFile.where(
-            type:,
+            type: period_type,
             period_year: year,
             period_month: month,
             format: format.to_s,
           ).take
         end
+
+        def validate_params!
+          if period_type.nil?
+            raise ArgumentError,
+                  "Invalid file type. Expected one of: #{ExchangeRateFile::APPLICABLE_TYPES.join(', ')}"
+          end
+        end
       end
     end
   end

app/models/exchange_rate_file.rb

@@ -1,5 +1,5 @@
 class ExchangeRateFile < Sequel::Model
-  APPLICABLE_TYPES = %w[monthly_csv monthly_xml spot_csv average_csv].freeze
+  APPLICABLE_TYPES = %w[monthly_csv_hmrc monthly_csv monthly_xml spot_csv average_csv].freeze
   OBJECT_KEY_PREFIX = 'data/exchange_rates'.freeze
 
   include ContentAddressableId
@@ -25,10 +25,6 @@ def filename
   end
 
   dataset_module do
-    def applicable_types
-      where(type: APPLICABLE_TYPES)
-    end
-
     def by_year(period_year)
       where(period_year:)
     end

spec/requests/api/v2/exchange_rates/files_controller_spec.rb

@@ -5,16 +5,18 @@
     let(:data) { 'foo,bar\nqux,qul' }
 
     before do
-      create(:exchange_rate_file, type:, format:, period_year: year, period_month: month)
+      create(:exchange_rate_file, type: period_type, format:, period_year: year, period_month: month)
       allow(TariffSynchronizer::FileService).to receive(:get).and_call_original
-      allow(TariffSynchronizer::FileService).to receive(:get).with("data/exchange_rates/#{year}/#{month}/#{type}_#{year}-#{month}.#{format}").and_return(StringIO.new(data))
+      allow(TariffSynchronizer::FileService).to receive(:get)
+        .with("data/exchange_rates/#{year}/#{month}/#{period_type}_#{year}-#{month}.#{format}")
+        .and_return(StringIO.new(data))
     end
 
     context 'when requesting CSV format' do
-      let(:type) { 'monthly_csv' }
+      let(:period_type) { 'monthly_csv' }
       let(:format) { 'csv' }
 
-      before { get api_exchange_rates_file_path("#{type}_#{year}-#{month}", format: :csv) }
+      before { get api_exchange_rates_file_path("#{period_type}_#{year}-#{month}", format: :csv) }
 
       it 'returns HTTP status :ok' do
         expect(response).to have_http_status(:ok)
@@ -34,10 +36,10 @@
     end
 
     context 'when requesting XML format' do
-      let(:type) { 'monthly_xml' }
+      let(:period_type) { 'monthly_xml' }
       let(:format) { 'xml' }
 
-      before { get api_exchange_rates_file_path("#{type}_#{year}-#{month}", format: :xml) }
+      before { get api_exchange_rates_file_path("#{period_type}_#{year}-#{month}", format: :xml) }
 
       it 'returns HTTP status :ok' do
         expect(response).to have_http_status(:ok)
@@ -57,10 +59,10 @@
     end
 
     context 'when requesting HMRC CSV format' do
-      let(:type) { 'monthly_csv_hmrc' }
+      let(:period_type) { 'monthly_csv_hmrc' }
       let(:format) { 'csv' }
 
-      before { get api_exchange_rates_file_path("#{type}_#{year}-#{month}", format: :csv) }
+      before { get api_exchange_rates_file_path("#{period_type}_#{year}-#{month}", format: :csv) }
 
       it 'returns HTTP status :ok' do
         expect(response).to have_http_status(:ok)
@@ -79,15 +81,28 @@
       end
     end
 
-    context 'when requesting invalid type' do
-      let(:type) { 'non_existing_type' }
+    context 'when file not found' do
+      let(:period_type) { 'monthly_csv' }
       let(:format) { 'csv' }
 
-      before { get api_exchange_rates_file_path("#{type}_#{year}-#{month}", format: :csv) }
+      let(:wrong_year) { 2010 }
 
-      it 'returns HTTP status :not_found' do
+      before { get api_exchange_rates_file_path("#{period_type}_#{wrong_year}-#{month}", format: :csv) }
+
+      it 'returns HTTP 404  :not_found' do
         expect(response).to have_http_status(:not_found)
       end
     end
+
+    context 'when requesting invalid period type' do
+      let(:period_type) { 'invalid_period_type' }
+      let(:format) { 'csv' }
+
+      before { get api_exchange_rates_file_path("#{period_type}_#{year}-#{month}", format:) }
+
+      it 'returns HTTP 400 bad request' do
+        expect(response).to have_http_status(:bad_request)
+      end
+    end
   end
 end