diff --git a/app/lib/cds_synchronizer.rb b/app/lib/cds_synchronizer.rb index f32108d8a..c2adbd106 100644 --- a/app/lib/cds_synchronizer.rb +++ b/app/lib/cds_synchronizer.rb @@ -29,6 +29,8 @@ def download end def apply + raise WrongEnvironmentError unless TradeTariffBackend.uk? + check_tariff_updates_failures check_sequence @@ -64,6 +66,8 @@ def apply end def rollback(rollback_date, keep: false) + raise WrongEnvironmentError unless TradeTariffBackend.uk? + Rails.autoloaders.main.eager_load TradeTariffBackend.with_redis_lock do @@ -71,7 +75,6 @@ def rollback(rollback_date, keep: false) updates = TariffSynchronizer::CdsUpdate.where { issue_date > date } update_filenames = updates.pluck(:filename) - Sequel::Model.db.transaction do # Delete actual data oplog_based_models.each do |model| diff --git a/app/lib/taric_importer.rb b/app/lib/taric_importer.rb index dfe5935de..2f2cde282 100644 --- a/app/lib/taric_importer.rb +++ b/app/lib/taric_importer.rb @@ -22,13 +22,12 @@ def initialize(taric_update) end def import - filename = determine_filename(@taric_update.file_path) - return unless proceed_with_import?(filename) - handler = XmlProcessor.new(@taric_update.issue_date) + file = TariffSynchronizer::FileService.file_as_stringio(@taric_update) XmlParser::Reader.new(file, 'record', handler).parse - post_import(file_path: @taric_update.file_path, filename:) + + Rails.logger.info "Successfully imported Taric file: #{@taric_update.filename}" end class XmlProcessor @@ -54,41 +53,4 @@ def taric_failed_log(exception, hash) end end end - - private - - def proceed_with_import?(filename) - return true unless TradeTariffBackend.use_cds? - - TariffSynchronizer::TaricUpdate.find(filename: filename[0, 30]).blank? - end - - def post_import(file_path:, filename:) - create_update_entry(file_path:, filename:) if TradeTariffBackend.use_cds? - Rails.logger.info "Successfully imported Taric file: #{@taric_update.filename}" - end - - def create_update_entry(file_path:, filename:) - file_size = determine_file_size(file_path) - issue_date = Date.parse(filename.scan(/[0-9]{8}/).last) - TariffSynchronizer::TaricUpdate.find_or_create( - filename: filename[0, 30], - issue_date:, - filesize: file_size, - state: 'A', - applied_at: Time.zone.now, - updated_at: Time.zone.now, - ) - end - - def determine_file_size(file_path) - file_size = File.size(file_path) - return file_size if file_size <= 214_748_364_7 - - file_size / 1024 - end - - def determine_filename(file_path) - File.basename(file_path) - end end diff --git a/app/lib/taric_synchronizer.rb b/app/lib/taric_synchronizer.rb index 85ea9018a..0ed343548 100644 --- a/app/lib/taric_synchronizer.rb +++ b/app/lib/taric_synchronizer.rb @@ -47,6 +47,8 @@ def download end def apply + raise WrongEnvironmentError unless TradeTariffBackend.xi? + check_tariff_updates_failures check_sequence @@ -78,6 +80,8 @@ def apply # # NOTE: this does not remove records from initial seed def rollback(rollback_date, keep: false) + raise WrongEnvironmentError unless TradeTariffBackend.xi? + TradeTariffBackend.with_redis_lock do date = Date.parse(rollback_date.to_s) @@ -122,7 +126,7 @@ def rollback(rollback_date, keep: false) end end - Rails.logger.info "Rolled back to #{date}. Forced keeping records: #{!!keep}" + Rails.logger.info "Rolled back to #{date}. Forced keeping records: #{!keep.nil?}" end rescue Redlock::LockError Rails.logger.warn("Failed to acquire Redis lock for rollback to #{rollback_date}. Keep records: #{keep}") diff --git a/app/lib/tariff_synchronizer.rb b/app/lib/tariff_synchronizer.rb index 65ea9e717..d8b0fd955 100644 --- a/app/lib/tariff_synchronizer.rb +++ b/app/lib/tariff_synchronizer.rb @@ -1,5 +1,6 @@ module TariffSynchronizer class FailedUpdatesError < StandardError; end + class WrongEnvironmentError < StandardError; end delegate :instrument, :subscribe, to: ActiveSupport::Notifications diff --git a/app/lib/trade_tariff_backend.rb b/app/lib/trade_tariff_backend.rb index 7f17fd51e..a12d94ccb 100644 --- a/app/lib/trade_tariff_backend.rb +++ b/app/lib/trade_tariff_backend.rb @@ -39,10 +39,6 @@ def admin_email ENV.fetch('TARIFF_SYNC_EMAIL') end - def use_cds? - ENV['CDS'] == 'true' - end - def patch_broken_taric_downloads? ENV['PATCH_BROKEN_TARIC_DOWNLOADS'] == 'true' end @@ -168,7 +164,7 @@ def check_query_count? end def excess_query_threshold - @excess_query_threshold ||= ENV['EXCESS_QUERY_THRESHOLD'].presence&.to_i || 0 + @excess_query_threshold ||= ENV['EXCESS_QUERY_THRESHOLD'].presence.to_i end def api_version(request) diff --git a/app/workers/apply_worker.rb b/app/workers/apply_worker.rb index 1273a0c2e..bb5fdddb9 100644 --- a/app/workers/apply_worker.rb +++ b/app/workers/apply_worker.rb @@ -4,7 +4,7 @@ class ApplyWorker sidekiq_options queue: :rollbacks, retry: false def perform - if TradeTariffBackend.use_cds? + if TradeTariffBackend.uk? CdsSynchronizer.apply else TaricSynchronizer.apply diff --git a/app/workers/download_worker.rb b/app/workers/download_worker.rb index a3e195813..f64fa11ac 100644 --- a/app/workers/download_worker.rb +++ b/app/workers/download_worker.rb @@ -4,7 +4,7 @@ class DownloadWorker sidekiq_options queue: :rollbacks, retry: false def perform - if TradeTariffBackend.use_cds? + if TradeTariffBackend.uk? CdsSynchronizer.download else TaricSynchronizer.download diff --git a/app/workers/rollback_worker.rb b/app/workers/rollback_worker.rb index 1381efdba..2a6795946 100644 --- a/app/workers/rollback_worker.rb +++ b/app/workers/rollback_worker.rb @@ -4,7 +4,7 @@ class RollbackWorker sidekiq_options queue: :rollbacks, retry: false def perform(date, redownload = false) - if TradeTariffBackend.use_cds? + if TradeTariffBackend.uk? CdsSynchronizer.rollback(date, keep: redownload) else TaricSynchronizer.rollback(date, keep: redownload) diff --git a/lib/tasks/tariff.rake b/lib/tasks/tariff.rake index 5b80bfcd6..429fc04c1 100644 --- a/lib/tasks/tariff.rake +++ b/lib/tasks/tariff.rake @@ -33,7 +33,7 @@ namespace :tariff do namespace :sync do desc 'Update database by downloading and then applying TARIC or CDS updates via worker' task download_apply_and_reindex: %i[environment class_eager_load] do - if TradeTariffBackend.use_cds? + if TradeTariffBackend.uk? CdsUpdatesSynchronizerWorker.perform_async(true, true) else TaricUpdatesSynchronizerWorker.perform_async(true) @@ -42,7 +42,7 @@ namespace :tariff do desc 'Download pending Taric or CDS update files, Update tariff_updates table' task download: %i[environment class_eager_load] do - if TradeTariffBackend.use_cds? + if TradeTariffBackend.uk? CdsSynchronizer.download else TaricSynchronizer.download @@ -51,7 +51,7 @@ namespace :tariff do desc 'Apply pending updates for Taric or CDS' task apply: %i[environment class_eager_load] do - if TradeTariffBackend.use_cds? + if TradeTariffBackend.uk? CdsSynchronizer.apply else TaricSynchronizer.apply @@ -61,7 +61,7 @@ namespace :tariff do desc 'Rollback to specific date in the past' task rollback: %w[environment class_eager_load] do if ENV['DATE'] - if TradeTariffBackend.use_cds? + if TradeTariffBackend.uk? CdsSynchronizer.rollback(ENV['DATE'], keep: ENV['KEEP']) else TaricSynchronizer.rollback(ENV['DATE'], keep: ENV['KEEP']) diff --git a/spec/integration/taric_synchronizer_spec.rb b/spec/integration/taric_synchronizer_spec.rb index e1d9f4e0e..8dc3e88db 100644 --- a/spec/integration/taric_synchronizer_spec.rb +++ b/spec/integration/taric_synchronizer_spec.rb @@ -46,8 +46,6 @@ allow(instance).to receive( :persist, ).and_raise OpenSearch::Transport::Transport::SnifferTimeoutError - - allow(TariffSynchronizer::TaricUpdate).to receive(:find).and_return(nil) end it 'stops syncing' do @@ -67,8 +65,6 @@ allow(instance).to receive( :persist, ).and_raise Timeout::Error - - allow(TariffSynchronizer::TaricUpdate).to receive(:find).and_return(nil) end it 'stops syncing' do @@ -175,13 +171,13 @@ end describe '#apply', truncation: true do - xit 'raises a wrong environment error' do + it 'raises a wrong environment error' do expect { described_class.apply }.to raise_error TariffSynchronizer::WrongEnvironmentError end end describe '#rollback', truncation: true do - xit 'raises a wrong environment error' do + it 'raises a wrong environment error' do expect { described_class.rollback(Time.zone.yesterday, keep: true) }.to raise_error TariffSynchronizer::WrongEnvironmentError end end diff --git a/spec/unit/cds_synchronizer_spec.rb b/spec/unit/cds_synchronizer_spec.rb index 0547fa7ac..ad766e609 100644 --- a/spec/unit/cds_synchronizer_spec.rb +++ b/spec/unit/cds_synchronizer_spec.rb @@ -109,7 +109,7 @@ end context 'with xi service' do - xit 'raises a wrong environment error' do + it 'raises a wrong environment error' do allow(TradeTariffBackend).to receive(:service).and_return('xi') expect { described_class.apply }.to raise_error TariffSynchronizer::WrongEnvironmentError end @@ -126,7 +126,7 @@ end context 'with xi service' do - xit 'raises a wrong environment error' do + it 'raises a wrong environment error' do allow(TradeTariffBackend).to receive(:service).and_return('xi') expect { described_class.rollback(Time.zone.yesterday, keep: true) }.to raise_error TariffSynchronizer::WrongEnvironmentError end diff --git a/spec/unit/taric_synchronizer_spec.rb b/spec/unit/taric_synchronizer_spec.rb index dafa19aa3..f5795494e 100644 --- a/spec/unit/taric_synchronizer_spec.rb +++ b/spec/unit/taric_synchronizer_spec.rb @@ -101,6 +101,7 @@ allow_any_instance_of(TaricImporter).to receive(:import) allow(TariffSynchronizer::TariffLogger).to receive(:failed_update) allow(TradeTariffBackend).to receive(:service).and_return('xi') + allow(TradeTariffBackend).to receive(:service).and_return('xi') applied_update pending_update end @@ -148,6 +149,7 @@ before do failed_update allow(TradeTariffBackend).to receive(:service).and_return('xi') + allow(TradeTariffBackend).to receive(:service).and_return('xi') end it 'does not apply pending updates' do @@ -171,7 +173,7 @@ end context 'with uk service' do - xit 'will raise an wrong environment error' do + it 'will raise an wrong environment error' do allow(TradeTariffBackend).to receive(:service).and_return('uk') expect { described_class.apply }.to raise_error TariffSynchronizer::WrongEnvironmentError end @@ -226,7 +228,7 @@ end context 'with uk service' do - xit 'will raise an wrong environment error' do + it 'will raise an wrong environment error' do allow(TradeTariffBackend).to receive(:service).and_return('uk') expect { described_class.rollback(Time.zone.yesterday, keep: true) }.to raise_error TariffSynchronizer::WrongEnvironmentError end diff --git a/spec/unit/tariff_synchronizer/base_update_importer_spec.rb b/spec/unit/tariff_synchronizer/base_update_importer_spec.rb index 18fc3efb7..9d3f95126 100644 --- a/spec/unit/tariff_synchronizer/base_update_importer_spec.rb +++ b/spec/unit/tariff_synchronizer/base_update_importer_spec.rb @@ -6,6 +6,9 @@ describe '#apply', truncation: true do before do allow(TradeTariffBackend).to receive(:service).and_return('xi') + # Setting :FIND to NIL as Taric_Importer.rb checks a file doesn't exist to Proceed_With_Import? + # SEE SPECS ON LINES 39 & 58. + allow(TariffSynchronizer::TaricUpdate).to receive(:find).and_return(nil) # Assuming no existing record end it 'calls the import! method to the object' do diff --git a/spec/workers/rollback_worker_spec.rb b/spec/workers/rollback_worker_spec.rb index 982ad0e31..522380f4b 100644 --- a/spec/workers/rollback_worker_spec.rb +++ b/spec/workers/rollback_worker_spec.rb @@ -31,7 +31,6 @@ context 'for uk' do before do - allow(TradeTariffBackend).to receive(:use_cds?).and_return('true') allow(TradeTariffBackend).to receive(:service).and_return('uk') allow(TaricSynchronizer).to receive(:rollback) allow(CdsSynchronizer).to receive(:rollback).with(date, keep: false) diff --git a/terraform/admin_uk.tf b/terraform/admin_uk.tf index b4a4c85c1..c709a34db 100644 --- a/terraform/admin_uk.tf +++ b/terraform/admin_uk.tf @@ -36,10 +36,6 @@ module "backend_admin_uk" { local.backend_common_vars, local.backend_common_worker_vars, [ - { - name = "CDS" - value = "true" - }, { name = "GOVUK_APP_DOMAIN" value = "tariff-uk-admin-${var.environment}.apps.internal" # This is necessary for a GOVUK gem we're not using diff --git a/terraform/admin_xi.tf b/terraform/admin_xi.tf index d2e543317..c9586d0bb 100644 --- a/terraform/admin_xi.tf +++ b/terraform/admin_xi.tf @@ -36,10 +36,6 @@ module "backend_admin_xi" { local.backend_common_vars, local.backend_common_worker_vars, [ - { - name = "CDS" - value = "false" - }, { name = "GOVUK_APP_DOMAIN" value = "tariff-xi-admin-${var.environment}.apps.internal" # This is necessary for a GOVUK gem we're not using diff --git a/terraform/backend_uk.tf b/terraform/backend_uk.tf index c3dba3d03..68b05bba6 100644 --- a/terraform/backend_uk.tf +++ b/terraform/backend_uk.tf @@ -37,10 +37,6 @@ module "backend_uk" { service_environment_config = flatten([ local.backend_common_vars, [ - { - name = "CDS" - value = "true" - }, { name = "GOVUK_APP_DOMAIN" value = "tariff-uk-backend-${var.environment}.apps.internal" # This is necessary for a GOVUK gem we're not using diff --git a/terraform/backend_xi.tf b/terraform/backend_xi.tf index 42280d5af..e22ca595e 100644 --- a/terraform/backend_xi.tf +++ b/terraform/backend_xi.tf @@ -35,10 +35,6 @@ module "backend_xi" { service_environment_config = flatten([local.backend_common_vars, [ - { - name = "CDS" - value = "false" - }, { name = "GOVUK_APP_DOMAIN" value = "tariff-xi-backend-${var.environment}.apps.internal" # This is necessary for a GOVUK gem we're not using diff --git a/terraform/worker_uk.tf b/terraform/worker_uk.tf index a96241caa..9422fe1e3 100644 --- a/terraform/worker_uk.tf +++ b/terraform/worker_uk.tf @@ -47,10 +47,6 @@ module "worker_uk" { local.backend_common_vars, local.backend_common_worker_vars, [ - { - name = "CDS" - value = "true" - }, { name = "GOVUK_APP_DOMAIN" value = "tariff-uk-worker-${var.environment}.apps.internal" # This is necessary for a GOVUK gem we're not using diff --git a/terraform/worker_xi.tf b/terraform/worker_xi.tf index ebfd68b08..dd5701b81 100644 --- a/terraform/worker_xi.tf +++ b/terraform/worker_xi.tf @@ -47,10 +47,6 @@ module "worker_xi" { local.backend_common_vars, local.backend_common_worker_vars, [ - { - name = "CDS" - value = "false" - }, { name = "GOVUK_APP_DOMAIN" value = "tariff-xi-worker-${var.environment}.apps.internal" # This is necessary for a GOVUK gem we're not using