From 5c12a4f604394b08a13e03912abfbd16948de49d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Meny?= Date: Tue, 9 Aug 2022 10:49:39 +0100 Subject: [PATCH] CLDC-1219: Single import task (#821) --- app/services/imports/user_import_service.rb | 2 +- app/services/storage_service.rb | 5 ++ lib/tasks/data_import.rake | 2 +- lib/tasks/full_import.rake | 30 ++++++++ spec/lib/tasks/full_import_spec.rb | 85 +++++++++++++++++++++ spec/services/storage_service_spec.rb | 67 ++++++++++++---- 6 files changed, 174 insertions(+), 17 deletions(-) create mode 100644 lib/tasks/full_import.rake create mode 100644 spec/lib/tasks/full_import_spec.rb diff --git a/app/services/imports/user_import_service.rb b/app/services/imports/user_import_service.rb index aec227f54..1d1fbc84e 100644 --- a/app/services/imports/user_import_service.rb +++ b/app/services/imports/user_import_service.rb @@ -13,8 +13,8 @@ module Imports def create_user(xml_document) organisation = Organisation.find_by(old_org_id: user_field_value(xml_document, "institution")) old_user_id = user_field_value(xml_document, "id") - name = user_field_value(xml_document, "full-name") email = user_field_value(xml_document, "email").downcase.strip + name = user_field_value(xml_document, "full-name") || email deleted = user_field_value(xml_document, "deleted") if User.find_by(old_user_id:, organisation:) diff --git a/app/services/storage_service.rb b/app/services/storage_service.rb index 1e9e374ce..766af69fa 100644 --- a/app/services/storage_service.rb +++ b/app/services/storage_service.rb @@ -13,6 +13,11 @@ class StorageService .flat_map { |response| response.contents.map(&:key) } end + def folder_present?(folder) + response = @client.list_objects_v2(bucket: @configuration.bucket_name, prefix: folder, max_keys: 1) + response.key_count == 1 + end + def get_file_io(file_name) @client.get_object(bucket: @configuration.bucket_name, key: file_name) .body diff --git a/lib/tasks/data_import.rake b/lib/tasks/data_import.rake index 54ebe60d4..94553f0da 100644 --- a/lib/tasks/data_import.rake +++ b/lib/tasks/data_import.rake @@ -1,5 +1,5 @@ namespace :core do - desc "Import data XMLs from Softwire system" + desc "Import data XMLs from legacy CORE" task :data_import, %i[type path] => :environment do |_task, args| type = args[:type] path = args[:path] diff --git a/lib/tasks/full_import.rake b/lib/tasks/full_import.rake new file mode 100644 index 000000000..42ac083f1 --- /dev/null +++ b/lib/tasks/full_import.rake @@ -0,0 +1,30 @@ +Import = Struct.new("Import", :import_class, :import_method, :folder) + +namespace :core do + desc "Import all data XMLs from legacy CORE" + task :full_import, %i[path] => :environment do |_task, args| + path = args[:path] + raise "Usage: rake core:full_import['path/to/main_folder']" if path.blank? + + storage_service = StorageService.new(PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) + + import_list = [ + Import.new(Imports::OrganisationImportService, :create_organisations, "institution"), + Import.new(Imports::SchemeImportService, :create_schemes, "mgmtgroups"), + Import.new(Imports::SchemeLocationImportService, :create_scheme_locations, "schemes"), + Import.new(Imports::UserImportService, :create_users, "user"), + Import.new(Imports::DataProtectionConfirmationImportService, :create_data_protection_confirmations, "dataprotect"), + Import.new(Imports::OrganisationRentPeriodImportService, :create_organisation_rent_periods, "rent-period"), + Import.new(Imports::CaseLogsImportService, :create_logs, "logs"), + ] + + import_list.each do |import| + folder_path = File.join(path, import.folder, "") + if storage_service.folder_present?(folder_path) + import.import_class.new(storage_service).send(import.import_method, folder_path) + else + Rails.logger.info("#{folder_path} does not exist, skipping #{import.import_class}") + end + end + end +end diff --git a/spec/lib/tasks/full_import_spec.rb b/spec/lib/tasks/full_import_spec.rb new file mode 100644 index 000000000..360fb8516 --- /dev/null +++ b/spec/lib/tasks/full_import_spec.rb @@ -0,0 +1,85 @@ +require "rails_helper" +require "rake" + +describe "rake core:full_import", type: :task do + subject(:task) { Rake::Task["core:full_import"] } + + let(:instance_name) { "paas_import_instance" } + let(:storage_service) { instance_double(StorageService) } + let(:paas_config_service) { instance_double(PaasConfigurationService) } + + before do + Rake.application.rake_require("tasks/full_import") + Rake::Task.define_task(:environment) + task.reenable + + allow(StorageService).to receive(:new).and_return(storage_service) + allow(PaasConfigurationService).to receive(:new).and_return(paas_config_service) + allow(ENV).to receive(:[]) + allow(ENV).to receive(:[]).with("IMPORT_PAAS_INSTANCE").and_return(instance_name) + end + + context "when starting a full import" do + let(:fixture_path) { "spec/fixtures/imports" } + let(:case_logs_service) { instance_double(Imports::CaseLogsImportService) } + let(:rent_period_service) { instance_double(Imports::OrganisationRentPeriodImportService) } + let(:data_protection_service) { instance_double(Imports::DataProtectionConfirmationImportService) } + let(:user_service) { instance_double(Imports::UserImportService) } + let(:location_service) { instance_double(Imports::SchemeLocationImportService) } + let(:scheme_service) { instance_double(Imports::SchemeImportService) } + let(:organisation_service) { instance_double(Imports::OrganisationImportService) } + + before do + allow(Imports::OrganisationImportService).to receive(:new).and_return(organisation_service) + allow(Imports::SchemeImportService).to receive(:new).and_return(scheme_service) + allow(Imports::SchemeLocationImportService).to receive(:new).and_return(location_service) + allow(Imports::UserImportService).to receive(:new).and_return(user_service) + allow(Imports::DataProtectionConfirmationImportService).to receive(:new).and_return(data_protection_service) + allow(Imports::OrganisationRentPeriodImportService).to receive(:new).and_return(rent_period_service) + allow(Imports::CaseLogsImportService).to receive(:new).and_return(case_logs_service) + end + + it "raises an exception if no parameters are provided" do + expect { task.invoke }.to raise_error(/Usage/) + end + + context "with all folders being present" do + before { allow(storage_service).to receive(:folder_present?).and_return(true) } + + it "calls every import method with the correct folder" do + expect(organisation_service).to receive(:create_organisations).with("#{fixture_path}/institution/") + expect(scheme_service).to receive(:create_schemes).with("#{fixture_path}/mgmtgroups/") + expect(location_service).to receive(:create_scheme_locations).with("#{fixture_path}/schemes/") + expect(user_service).to receive(:create_users).with("#{fixture_path}/user/") + expect(data_protection_service).to receive(:create_data_protection_confirmations).with("#{fixture_path}/dataprotect/") + expect(rent_period_service).to receive(:create_organisation_rent_periods).with("#{fixture_path}/rent-period/") + expect(case_logs_service).to receive(:create_logs).with("#{fixture_path}/logs/") + + task.invoke(fixture_path) + end + end + + context "when a specific folders are missing" do + before do + allow(storage_service).to receive(:folder_present?).and_return(true) + allow(storage_service).to receive(:folder_present?).with("#{fixture_path}/mgmtgroups/").and_return(false) + allow(storage_service).to receive(:folder_present?).with("#{fixture_path}/schemes/").and_return(false) + end + + it "only calls import methods for existing folders" do + expect(organisation_service).to receive(:create_organisations) + expect(user_service).to receive(:create_users) + expect(data_protection_service).to receive(:create_data_protection_confirmations) + expect(rent_period_service).to receive(:create_organisation_rent_periods) + expect(case_logs_service).to receive(:create_logs) + + expect(scheme_service).not_to receive(:create_schemes) + expect(location_service).not_to receive(:create_scheme_locations) + expect(Rails.logger).to receive(:info).with("spec/fixtures/imports/mgmtgroups/ does not exist, skipping Imports::SchemeImportService") + expect(Rails.logger).to receive(:info).with("spec/fixtures/imports/schemes/ does not exist, skipping Imports::SchemeLocationImportService") + + task.invoke(fixture_path) + end + end + end +end diff --git a/spec/services/storage_service_spec.rb b/spec/services/storage_service_spec.rb index 4e79228f6..d19d51ed8 100644 --- a/spec/services/storage_service_spec.rb +++ b/spec/services/storage_service_spec.rb @@ -99,7 +99,7 @@ RSpec.describe StorageService do end end - context "when we create a storage service and list files" do + context "when we create a storage service" do subject(:storage_service) { described_class.new(PaasConfigurationService.new, instance_name) } let(:s3_client_stub) { Aws::S3::Client.new(stub_responses: true) } @@ -110,22 +110,59 @@ RSpec.describe StorageService do allow(Aws::S3::Client).to receive(:new).and_return(s3_client_stub) end - it "returns a list with all present file names in a given folder" do - expected_filenames = %w[my_folder/my_file1.xml my_folder/my_file2.xml] - s3_client_stub.stub_responses(:list_objects_v2, { - contents: [ - { - key: expected_filenames[0], - }, - { - key: expected_filenames[1], - }, - ], - }) + context "and we list files based on a prefix" do + let(:expected_filenames) { %w[my_folder/my_file1.xml my_folder/my_file2.xml] } + + before do + s3_client_stub.stub_responses(:list_objects_v2, { + contents: [ + { + key: expected_filenames[0], + }, + { + key: expected_filenames[1], + }, + ], + }) + end + + it "returns a list with all present file names in a given folder" do + filenames = storage_service.list_files("my_folder") + expect(filenames).to eq(expected_filenames) + end + end - filenames = storage_service.list_files("my_folder") + context "and we check for an existing folder" do + before do + expected_filenames = %w[my_folder/my_file1.xml] + s3_client_stub.stub_responses(:list_objects_v2, { + key_count: 1, + contents: [ + { + key: expected_filenames[0], + }, + ], + }) + end + + it "returns true" do + response = storage_service.folder_present?("my_folder") + expect(response).to be_truthy + end + end - expect(filenames).to eq(expected_filenames) + context "and we check for a folder that does not exists" do + before do + s3_client_stub.stub_responses(:list_objects_v2, { + key_count: 0, + contents: [], + }) + end + + it "returns false" do + response = storage_service.folder_present?("my_folder") + expect(response).to be_falsey + end end end end