CLDC-1221: Support reading import from zip archive (#829)

app/services/archive_storage_service.rb

@@ -0,0 +1,24 @@
+class ArchiveStorageService < StorageService
+  MAX_SIZE = 50 * (1024**2) # 50MiB
+
+  def initialize(archive_io)
+    super()
+    @archive = Zip::File.open_buffer(archive_io)
+  end
+
+  def list_files(folder)
+    @archive.glob(File.join(folder, "*.*"))
+            .map(&:name)
+  end
+
+  def folder_present?(folder)
+    !list_files(folder).empty?
+  end
+
+  def get_file_io(file_name)
+    entry = @archive.get_entry(file_name)
+    raise "File too large to be extracted" if entry.size > MAX_SIZE
+
+    entry.get_input_stream
+  end
+end

app/services/s3_storage_service.rb

@@ -0,0 +1,78 @@
+class S3StorageService < StorageService
+  attr_reader :configuration
+
+  def initialize(paas_config_service, paas_instance_name)
+    super()
+    @paas_config_service = paas_config_service
+    @paas_instance_name = (paas_instance_name || "").to_sym
+    @configuration = create_configuration
+    @client = create_client
+  end
+
+  def list_files(folder)
+    @client.list_objects_v2(bucket: @configuration.bucket_name, prefix: folder)
+           .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
+  end
+
+  def write_file(file_name, data)
+    @client.put_object(
+      body: data,
+      bucket: @configuration.bucket_name,
+      key: file_name,
+    )
+  end
+
+private
+
+  def create_configuration
+    unless @paas_config_service.config_present?
+      raise "No PaaS configuration present"
+    end
+    unless @paas_config_service.s3_buckets.key?(@paas_instance_name)
+      raise "#{@paas_instance_name} instance name could not be found"
+    end
+
+    bucket_config = @paas_config_service.s3_buckets[@paas_instance_name]
+    StorageConfiguration.new(bucket_config[:credentials])
+  end
+
+  def create_client
+    credentials =
+      Aws::Credentials.new(
+        @configuration.access_key_id,
+        @configuration.secret_access_key,
+      )
+    Aws::S3::Client.new(
+      region: @configuration.region,
+      credentials:,
+    )
+  end
+end
+
+class StorageConfiguration
+  attr_reader :access_key_id, :secret_access_key, :bucket_name, :region
+
+  def initialize(credentials)
+    @access_key_id = credentials[:aws_access_key_id]
+    @secret_access_key = credentials[:aws_secret_access_key]
+    @bucket_name = credentials[:bucket_name]
+    @region = credentials[:aws_region]
+  end
+
+  def ==(other)
+    @access_key_id == other.access_key_id &&
+      @secret_access_key == other.secret_access_key &&
+      @bucket_name == other.bucket_name &&
+      @region == other.region
+  end
+end

app/services/storage_service.rb

@@ -1,77 +1,17 @@
 class StorageService
-  attr_reader :configuration
+  def list_files(_folder)
-
+    raise NotImplementedError
-  def initialize(paas_config_service, paas_instance_name)
-    @paas_config_service = paas_config_service
-    @paas_instance_name = (paas_instance_name || "").to_sym
-    @configuration = create_configuration
-    @client = create_client
-  end
-
-  def list_files(folder)
-    @client.list_objects_v2(bucket: @configuration.bucket_name, prefix: folder)
-           .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
   end
 
-  def write_file(file_name, data)
+  def folder_present?(_folder)
-    @client.put_object(
+    raise NotImplementedError
-      body: data,
-      bucket: @configuration.bucket_name,
-      key: file_name,
-    )
   end
 
-private
+  def get_file_io(_file_name)
-
+    raise NotImplementedError
-  def create_configuration
-    unless @paas_config_service.config_present?
-      raise "No PaaS configuration present"
-    end
-    unless @paas_config_service.s3_buckets.key?(@paas_instance_name)
-      raise "#{@paas_instance_name} instance name could not be found"
-    end
-
-    bucket_config = @paas_config_service.s3_buckets[@paas_instance_name]
-    StorageConfiguration.new(bucket_config[:credentials])
-  end
-
-  def create_client
-    credentials =
-      Aws::Credentials.new(
-        @configuration.access_key_id,
-        @configuration.secret_access_key,
-      )
-    Aws::S3::Client.new(
-      region: @configuration.region,
-      credentials:,
-    )
-  end
-end
-
-class StorageConfiguration
-  attr_reader :access_key_id, :secret_access_key, :bucket_name, :region
-
-  def initialize(credentials)
-    @access_key_id = credentials[:aws_access_key_id]
-    @secret_access_key = credentials[:aws_secret_access_key]
-    @bucket_name = credentials[:bucket_name]
-    @region = credentials[:aws_region]
   end
 
-  def ==(other)
+  def write_file(_file_name, _data)
-    @access_key_id == other.access_key_id &&
+    raise NotImplementedError
-      @secret_access_key == other.secret_access_key &&
-      @bucket_name == other.bucket_name &&
-      @region == other.region
   end
 end

lib/tasks/data_export.rake

@@ -4,7 +4,7 @@ namespace :core do
     format = args[:format]
     full_update = args[:full_update].present? && args[:full_update] == "true"
 
-    storage_service = StorageService.new(PaasConfigurationService.new, ENV["EXPORT_PAAS_INSTANCE"])
+    storage_service = S3StorageService.new(PaasConfigurationService.new, ENV["EXPORT_PAAS_INSTANCE"])
     export_service = Exports::CaseLogExportService.new(storage_service)
 
     if format.present? && format == "CSV"

lib/tasks/data_import.rake

@@ -5,7 +5,7 @@ namespace :core do
     path = args[:path]
     raise "Usage: rake core:data_import['data_type', 'path/to/xml_files']" if path.blank? || type.blank?
 
-    storage_service = StorageService.new(PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    storage_service = S3StorageService.new(PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
 
     case type
     when "organisation"

lib/tasks/data_import_field.rake

@@ -5,7 +5,7 @@ namespace :core do
     path = args[:path]
     raise "Usage: rake core:data_import_field['field','path/to/xml_files']" if path.blank? || field.blank?
 
-    storage_service = StorageService.new(PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    storage_service = S3StorageService.new(PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
 
     # We only allow a reduced list of known fields to be updatable
     case field

lib/tasks/full_import.rake

@@ -2,11 +2,13 @@ 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|
+  task :full_import, %i[archive_path] => :environment do |_task, args|
-    path = args[:path]
+    archive_path = args[:archive_path]
-    raise "Usage: rake core:full_import['path/to/main_folder']" if path.blank?
+    raise "Usage: rake core:full_import['path/to/archive']" if archive_path.blank?
 
-    storage_service = StorageService.new(PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    s3_service = S3StorageService.new(PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    archive_io = s3_service.get_file_io(archive_path)
+    archive_service = ArchiveStorageService.new(archive_io)
 
     import_list = [
       Import.new(Imports::OrganisationImportService, :create_organisations, "institution"),
@@ -18,12 +20,11 @@ namespace :core do
       Import.new(Imports::CaseLogsImportService, :create_logs, "logs"),
     ]
 
-    import_list.each do |import|
+    import_list.each do |step|
-      folder_path = File.join(path, import.folder, "")
+      if archive_service.folder_present?(step.folder)
-      if storage_service.folder_present?(folder_path)
+        step.import_class.new(archive_service).send(step.import_method, step.folder)
-        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}")
+        Rails.logger.info("#{step.folder} does not exist, skipping #{step.import_class}")
       end
     end
   end

spec/lib/tasks/data_export_spec.rb

@@ -5,7 +5,7 @@ describe "rake core:data_export", type: task do
   subject(:task) { Rake::Task["core:data_export"] }
 
   let(:paas_instance) { "paas_export_instance" }
-  let(:storage_service) { instance_double(StorageService) }
+  let(:storage_service) { instance_double(S3StorageService) }
   let(:paas_config_service) { instance_double(PaasConfigurationService) }
   let(:export_service) { instance_double(Exports::CaseLogExportService) }
 
@@ -14,7 +14,7 @@ describe "rake core:data_export", type: task do
     Rake::Task.define_task(:environment)
     task.reenable
 
-    allow(StorageService).to receive(:new).and_return(storage_service)
+    allow(S3StorageService).to receive(:new).and_return(storage_service)
     allow(PaasConfigurationService).to receive(:new).and_return(paas_config_service)
     allow(Exports::CaseLogExportService).to receive(:new).and_return(export_service)
     allow(ENV).to receive(:[])
@@ -23,7 +23,7 @@ describe "rake core:data_export", type: task do
 
   context "when exporting case logs with no parameters" do
     it "starts the XML export process" do
-      expect(StorageService).to receive(:new).with(paas_config_service, paas_instance)
+      expect(S3StorageService).to receive(:new).with(paas_config_service, paas_instance)
       expect(Exports::CaseLogExportService).to receive(:new).with(storage_service)
       expect(export_service).to receive(:export_xml_case_logs)
 
@@ -33,7 +33,7 @@ describe "rake core:data_export", type: task do
 
   context "when exporting case logs with CSV format" do
     it "starts the CSV export process" do
-      expect(StorageService).to receive(:new).with(paas_config_service, paas_instance)
+      expect(S3StorageService).to receive(:new).with(paas_config_service, paas_instance)
       expect(Exports::CaseLogExportService).to receive(:new).with(storage_service)
       expect(export_service).to receive(:export_csv_case_logs)
 

spec/lib/tasks/data_import_spec.rb

@@ -5,7 +5,7 @@ describe "rake core:data_import", type: :task do
   subject(:task) { Rake::Task["core:data_import"] }
 
   let(:instance_name) { "paas_import_instance" }
-  let(:storage_service) { instance_double(StorageService) }
+  let(:storage_service) { instance_double(S3StorageService) }
   let(:paas_config_service) { instance_double(PaasConfigurationService) }
 
   before do
@@ -13,7 +13,7 @@ describe "rake core:data_import", type: :task do
     Rake::Task.define_task(:environment)
     task.reenable
 
-    allow(StorageService).to receive(:new).and_return(storage_service)
+    allow(S3StorageService).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)
@@ -29,7 +29,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates an organisation from the given XML file" do
-      expect(StorageService).to receive(:new).with(paas_config_service, instance_name)
+      expect(S3StorageService).to receive(:new).with(paas_config_service, instance_name)
       expect(Imports::OrganisationImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_organisations).with(fixture_path)
 
@@ -47,7 +47,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates a user from the given XML file" do
-      expect(StorageService).to receive(:new).with(paas_config_service, instance_name)
+      expect(S3StorageService).to receive(:new).with(paas_config_service, instance_name)
       expect(Imports::UserImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_users).with(fixture_path)
 
@@ -65,7 +65,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates an organisation from the given XML file" do
-      expect(StorageService).to receive(:new).with(paas_config_service, instance_name)
+      expect(S3StorageService).to receive(:new).with(paas_config_service, instance_name)
       expect(Imports::DataProtectionConfirmationImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_data_protection_confirmations).with(fixture_path)
 
@@ -83,7 +83,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates an organisation la from the given XML file" do
-      expect(StorageService).to receive(:new).with(paas_config_service, instance_name)
+      expect(S3StorageService).to receive(:new).with(paas_config_service, instance_name)
       expect(Imports::OrganisationRentPeriodImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_organisation_rent_periods).with(fixture_path)
 
@@ -101,7 +101,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates case logs from the given XML file" do
-      expect(StorageService).to receive(:new).with(paas_config_service, instance_name)
+      expect(S3StorageService).to receive(:new).with(paas_config_service, instance_name)
       expect(Imports::CaseLogsImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_logs).with(fixture_path)
 
@@ -119,7 +119,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates a scheme from the given XML file" do
-      expect(StorageService).to receive(:new).with(paas_config_service, instance_name)
+      expect(S3StorageService).to receive(:new).with(paas_config_service, instance_name)
       expect(Imports::SchemeImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_schemes).with(fixture_path)
 
@@ -137,7 +137,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates a scheme location from the given XML file" do
-      expect(StorageService).to receive(:new).with(paas_config_service, instance_name)
+      expect(S3StorageService).to receive(:new).with(paas_config_service, instance_name)
       expect(Imports::SchemeLocationImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_scheme_locations).with(fixture_path)
 

spec/lib/tasks/date_import_field_spec.rb

@@ -5,7 +5,7 @@ describe "rake core:data_import_field", type: :task do
   subject(:task) { Rake::Task["core:data_import_field"] }
 
   let(:instance_name) { "paas_import_instance" }
-  let(:storage_service) { instance_double(StorageService) }
+  let(:storage_service) { instance_double(S3StorageService) }
   let(:paas_config_service) { instance_double(PaasConfigurationService) }
 
   before do
@@ -13,7 +13,7 @@ describe "rake core:data_import_field", type: :task do
     Rake::Task.define_task(:environment)
     task.reenable
 
-    allow(StorageService).to receive(:new).and_return(storage_service)
+    allow(S3StorageService).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)
@@ -32,7 +32,7 @@ describe "rake core:data_import_field", type: :task do
       let(:field) { "tenant_code" }
 
       it "properly configures the storage service" do
-        expect(StorageService).to receive(:new).with(paas_config_service, instance_name)
+        expect(S3StorageService).to receive(:new).with(paas_config_service, instance_name)
         task.invoke(field, fixture_path)
       end
 
@@ -46,7 +46,7 @@ describe "rake core:data_import_field", type: :task do
       let(:field) { "lettings_allocation" }
 
       it "properly configures the storage service" do
-        expect(StorageService).to receive(:new).with(paas_config_service, instance_name)
+        expect(S3StorageService).to receive(:new).with(paas_config_service, instance_name)
         task.invoke(field, fixture_path)
       end
 
@@ -60,7 +60,7 @@ describe "rake core:data_import_field", type: :task do
       let(:field) { "major_repairs" }
 
       it "properly configures the storage service" do
-        expect(StorageService).to receive(:new).with(paas_config_service, instance_name)
+        expect(S3StorageService).to receive(:new).with(paas_config_service, instance_name)
         task.invoke(field, fixture_path)
       end
 

spec/lib/tasks/full_import_spec.rb

@@ -1,11 +1,12 @@
 require "rails_helper"
 require "rake"
+require "zip"
 
 describe "rake core:full_import", type: :task do
   subject(:task) { Rake::Task["core:full_import"] }
 
-  let(:instance_name) { "paas_import_instance" }
+  let(:s3_service) { instance_double(S3StorageService) }
-  let(:storage_service) { instance_double(StorageService) }
+  let(:archive_service) { instance_double(ArchiveStorageService) }
   let(:paas_config_service) { instance_double(PaasConfigurationService) }
 
   before do
@@ -13,14 +14,13 @@ describe "rake core:full_import", type: :task do
     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(S3StorageService).to receive(:new).and_return(s3_service)
-    allow(ENV).to receive(:[]).with("IMPORT_PAAS_INSTANCE").and_return(instance_name)
+    allow(s3_service).to receive(:get_file_io)
+    allow(ArchiveStorageService).to receive(:new).and_return(archive_service)
   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) }
@@ -44,16 +44,16 @@ describe "rake core:full_import", type: :task do
     end
 
     context "with all folders being present" do
-      before { allow(storage_service).to receive(:folder_present?).and_return(true) }
+      before { allow(archive_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(organisation_service).to receive(:create_organisations).with("institution")
-        expect(scheme_service).to receive(:create_schemes).with("#{fixture_path}/mgmtgroups/")
+        expect(scheme_service).to receive(:create_schemes).with("mgmtgroups")
-        expect(location_service).to receive(:create_scheme_locations).with("#{fixture_path}/schemes/")
+        expect(location_service).to receive(:create_scheme_locations).with("schemes")
-        expect(user_service).to receive(:create_users).with("#{fixture_path}/user/")
+        expect(user_service).to receive(:create_users).with("user")
-        expect(data_protection_service).to receive(:create_data_protection_confirmations).with("#{fixture_path}/dataprotect/")
+        expect(data_protection_service).to receive(:create_data_protection_confirmations).with("dataprotect")
-        expect(rent_period_service).to receive(:create_organisation_rent_periods).with("#{fixture_path}/rent-period/")
+        expect(rent_period_service).to receive(:create_organisation_rent_periods).with("rent-period")
-        expect(case_logs_service).to receive(:create_logs).with("#{fixture_path}/logs/")
+        expect(case_logs_service).to receive(:create_logs).with("logs")
 
         task.invoke(fixture_path)
       end
@@ -61,9 +61,9 @@ describe "rake core:full_import", type: :task do
 
     context "when a specific folders are missing" do
       before do
-        allow(storage_service).to receive(:folder_present?).and_return(true)
+        allow(archive_service).to receive(:folder_present?).and_return(true)
-        allow(storage_service).to receive(:folder_present?).with("#{fixture_path}/mgmtgroups/").and_return(false)
+        allow(archive_service).to receive(:folder_present?).with("mgmtgroups").and_return(false)
-        allow(storage_service).to receive(:folder_present?).with("#{fixture_path}/schemes/").and_return(false)
+        allow(archive_service).to receive(:folder_present?).with("schemes").and_return(false)
       end
 
       it "only calls import methods for existing folders" do
@@ -75,8 +75,8 @@ describe "rake core:full_import", type: :task do
 
         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("mgmtgroups does not exist, skipping Imports::SchemeImportService")
-        expect(Rails.logger).to receive(:info).with("spec/fixtures/imports/schemes/ does not exist, skipping Imports::SchemeLocationImportService")
+        expect(Rails.logger).to receive(:info).with("schemes does not exist, skipping Imports::SchemeLocationImportService")
 
         task.invoke(fixture_path)
       end

spec/services/archive_storage_service_spec.rb

@@ -0,0 +1,65 @@
+require "rails_helper"
+
+RSpec.describe ArchiveStorageService do
+  subject(:archive_service) { described_class.new(archive_content) }
+
+  let(:compressed_folder) { "my_directory" }
+  let(:compressed_filename) { "hello.txt" }
+  let(:compressed_filepath) { File.join(compressed_folder, compressed_filename) }
+  let(:compressed_file) do
+    file = Tempfile.new
+    file << "Hello World\n"
+    file.rewind
+    file
+  end
+  let(:archive_content) do
+    zip_file = Zip::File.open_buffer(StringIO.new)
+    zip_file.mkdir(compressed_folder)
+    zip_file.add(compressed_filepath, compressed_file)
+    zip_file.write_buffer
+  end
+
+  describe "#list_files" do
+    it "returns the list of files present in an existing folder" do
+      file_list = archive_service.list_files(compressed_folder)
+      expect(file_list).to contain_exactly(compressed_filepath)
+    end
+
+    it "returns an empty file list for an unknown folder" do
+      file_list = archive_service.list_files("random_folder")
+      expect(file_list).to be_empty
+    end
+  end
+
+  describe "#folder_present?" do
+    it "returns true if a folder in the archive exists" do
+      presence = archive_service.folder_present?(compressed_folder)
+      expect(presence).to be_truthy
+    end
+
+    it "returns false if a folder in the archive does not exist" do
+      presence = archive_service.folder_present?("random_folder")
+      expect(presence).to be_falsey
+    end
+  end
+
+  describe "#get_file_io" do
+    it "returns the file content if a file exists" do
+      content = archive_service.get_file_io(compressed_filepath)
+      expect(content.read).to eq(compressed_file.read)
+    end
+
+    it "raises an error if the file exists but is too large" do
+      archive = archive_service.instance_variable_get(:@archive)
+      allow(archive).to receive(:get_entry).and_return(Zip::Entry.new(nil, "", nil, nil, nil, nil, nil, 100_000_000, nil))
+
+      expect { archive_service.get_file_io(compressed_filepath) }
+        .to raise_error(RuntimeError, "File too large to be extracted")
+    end
+
+    it "raises an error if a file does not exist" do
+      expect { archive_service.get_file_io("random.zzz") }
+        .to raise_error(Errno::ENOENT)
+    end
+  end
+end

spec/services/exports/case_log_export_service_spec.rb

@@ -3,7 +3,7 @@ require "rails_helper"
 RSpec.describe Exports::CaseLogExportService do
   subject(:export_service) { described_class.new(storage_service) }
 
-  let(:storage_service) { instance_double(StorageService) }
+  let(:storage_service) { instance_double(S3StorageService) }
 
   let(:xml_export_file) { File.open("spec/fixtures/exports/general_needs_log.xml", "r:UTF-8") }
   let(:local_manifest_file) { File.open("spec/fixtures/exports/manifest.xml", "r:UTF-8") }

spec/services/imports/case_logs_field_import_service_spec.rb

@@ -3,11 +3,11 @@ require "rails_helper"
 RSpec.describe Imports::CaseLogsFieldImportService do
   subject(:import_service) { described_class.new(storage_service, logger) }
 
-  let(:storage_service) { instance_double(StorageService) }
+  let(:storage_service) { instance_double(S3StorageService) }
   let(:logger) { instance_double(ActiveSupport::Logger) }
 
   let(:real_2021_2022_form) { Form.new("config/forms/2021_2022.json", "2021_2022") }
-  let(:fixture_directory) { "spec/fixtures/imports/case_logs" }
+  let(:fixture_directory) { "spec/fixtures/imports/logs" }
 
   let(:case_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" }
   let(:case_log_file) { open_file(fixture_directory, case_log_id) }

spec/services/imports/case_logs_import_service_spec.rb

@@ -3,12 +3,12 @@ require "rails_helper"
 RSpec.describe Imports::CaseLogsImportService do
   subject(:case_log_service) { described_class.new(storage_service, logger) }
 
-  let(:storage_service) { instance_double(StorageService) }
+  let(:storage_service) { instance_double(S3StorageService) }
   let(:logger) { instance_double(ActiveSupport::Logger) }
 
   let(:real_2021_2022_form) { Form.new("config/forms/2021_2022.json", "2021_2022") }
   let(:real_2022_2023_form) { Form.new("config/forms/2022_2023.json", "2022_2023") }
-  let(:fixture_directory) { "spec/fixtures/imports/case_logs" }
+  let(:fixture_directory) { "spec/fixtures/imports/logs" }
 
   let(:organisation) { FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP") }
   let(:scheme1) { FactoryBot.create(:scheme, old_visible_id: 123, owning_organisation: organisation) }

spec/services/imports/data_protection_confirmation_import_service_spec.rb

@@ -1,11 +1,11 @@
 require "rails_helper"
 
 RSpec.describe Imports::DataProtectionConfirmationImportService do
-  let(:fixture_directory) { "spec/fixtures/imports/data_protection_confirmations" }
+  let(:fixture_directory) { "spec/fixtures/imports/dataprotect" }
   let(:old_org_id) { "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618" }
   let(:old_id) { old_org_id }
   let(:import_file) { File.open("#{fixture_directory}/#{old_id}.xml") }
-  let(:storage_service) { instance_double(StorageService) }
+  let(:storage_service) { instance_double(S3StorageService) }
   let(:logger) { instance_double(ActiveSupport::Logger) }
 
   context "when importing data protection confirmations" do

spec/services/imports/organisation_import_service_spec.rb

@@ -1,11 +1,11 @@
 require "rails_helper"
 
 RSpec.describe Imports::OrganisationImportService do
-  let(:storage_service) { instance_double(StorageService) }
+  let(:storage_service) { instance_double(S3StorageService) }
   let(:logger) { instance_double(Rails::Rack::Logger) }
   let(:folder_name) { "organisations" }
   let(:filenames) { %w[my_folder/my_file1.xml my_folder/my_file2.xml] }
-  let(:fixture_directory) { "spec/fixtures/imports/organisations" }
+  let(:fixture_directory) { "spec/fixtures/imports/institution" }
 
   def create_organisation_file(fixture_directory, visible_id, name = nil)
     file = File.open("#{fixture_directory}/7c5bd5fb549c09a2c55d7cb90d7ba84927e64618.xml")

spec/services/imports/organisation_rent_period_import_service_spec.rb

@@ -1,11 +1,11 @@
 require "rails_helper"
 
 RSpec.describe Imports::OrganisationRentPeriodImportService do
-  let(:fixture_directory) { "spec/fixtures/imports/organisation_rent_periods" }
+  let(:fixture_directory) { "spec/fixtures/imports/rent-period" }
   let(:old_org_id) { "44026acc7ed5c29516b26f2a5deb639e5e37966d" }
   let(:old_id) { "ebd22326d33e389e9f1bfd546979d2c05f9e68d6" }
   let(:import_file) { File.open("#{fixture_directory}/#{old_id}.xml") }
-  let(:storage_service) { instance_double(StorageService) }
+  let(:storage_service) { instance_double(S3StorageService) }
   let(:logger) { instance_double(ActiveSupport::Logger) }
 
   context "when importing organisation rent periods" do

spec/services/imports/scheme_import_service_spec.rb

@@ -3,10 +3,10 @@ require "rails_helper"
 RSpec.describe Imports::SchemeImportService do
   subject(:scheme_service) { described_class.new(storage_service, logger) }
 
-  let(:storage_service) { instance_double(StorageService) }
+  let(:storage_service) { instance_double(S3StorageService) }
   let(:logger) { instance_double(ActiveSupport::Logger) }
 
-  let(:fixture_directory) { "spec/fixtures/imports/schemes" }
+  let(:fixture_directory) { "spec/fixtures/imports/mgmtgroups" }
   let(:scheme_id) { "6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d" }
 
   let!(:owning_org) { FactoryBot.create(:organisation, old_org_id: "7c5bd5fb549c09z2c55d9cb90d7ba84927e64618") }

spec/services/imports/scheme_location_import_service_spec.rb

@@ -3,10 +3,10 @@ require "rails_helper"
 RSpec.describe Imports::SchemeLocationImportService do
   subject(:location_service) { described_class.new(storage_service, logger) }
 
-  let(:storage_service) { instance_double(StorageService) }
+  let(:storage_service) { instance_double(S3StorageService) }
   let(:logger) { instance_double(ActiveSupport::Logger) }
 
-  let(:fixture_directory) { "spec/fixtures/imports/scheme_locations" }
+  let(:fixture_directory) { "spec/fixtures/imports/schemes" }
   let(:first_location_id) { "0ae7ad6dc0f1cf7ef33c18cc8c108bebc1b4923e" }
   let(:second_location_id) { "0bb3836b70b4dd9903263d5a764a5c45b964a89d" }
 

spec/services/imports/user_import_service_spec.rb

@@ -1,11 +1,11 @@
 require "rails_helper"
 
 RSpec.describe Imports::UserImportService do
-  let(:fixture_directory) { "spec/fixtures/imports/users" }
+  let(:fixture_directory) { "spec/fixtures/imports/user" }
   let(:old_user_id) { "fc7625a02b24ae16162aa63ae7cb33feeec0c373" }
   let(:old_org_id) { "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618" }
   let(:user_file) { File.open("#{fixture_directory}/#{old_user_id}.xml") }
-  let(:storage_service) { instance_double(StorageService) }
+  let(:storage_service) { instance_double(S3StorageService) }
   let(:logger) { instance_double(ActiveSupport::Logger) }
   let(:notify_client) { instance_double(Notifications::Client) }
   let(:devise_notify_mailer) { DeviseNotifyMailer.new }

spec/services/s3_storage_service_spec.rb

@@ -1,6 +1,6 @@
 require "rails_helper"
 
-RSpec.describe StorageService do
+RSpec.describe S3StorageService do
   let(:instance_name) { "instance_1" }
   let(:bucket_name) { "bucket_1" }
   let(:vcap_services) do