Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SIMPLE-FORMS] fix: updates to S3 presigned URL and directory naming logic #19454

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,12 @@ def write_file(dir_path, file_name, content)
File.write(File.join(dir_path, file_name), content)
end

def unique_file_name(form_number, id)
"#{Time.zone.today.strftime('%-m.%d.%y')}_form_#{form_number}_vagov_#{id}"
def unique_file_name(form_number, id, date = Time.zone.today)
"#{date.strftime('%-m.%d.%y')}_form_#{form_number}_vagov_#{id}"
end

def dated_directory_name(form_number)
"#{Time.zone.today.strftime('%-m.%d.%y')}-Form#{form_number}"
def dated_directory_name(form_number, date = Time.zone.today)
"#{date.strftime('%-m.%d.%y')}-Form#{form_number}"
end

def write_manifest(row, path)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ class S3Client
include FileUtilities

class << self
def fetch_presigned_url(id, type: :submission)
new(id:).generate_presigned_url(type:)
def fetch_presigned_url(id, config:, type: :submission)
new(id:, config:, type:).retrieve_presigned_url
end
end

Expand Down Expand Up @@ -41,6 +41,12 @@ def upload
config.handle_error("Failed #{upload_type} upload: #{id}", e)
end

def retrieve_presigned_url
archive = config.submission_archive_class.new(config:, id:, type: upload_type)
@archive_path, @manifest_row = archive.retrieval_data
generate_presigned_url(type: upload_type)
end

private

attr_reader :archive_path, :config, :id, :manifest_row, :parent_dir, :temp_directory_path, :upload_type
Expand Down Expand Up @@ -72,8 +78,7 @@ def update_manifest
temp_dir = Rails.root.join("tmp/#{SecureRandom.hex}-manifest/").to_s
create_directory!(temp_dir)
begin
form_number = manifest_row[1]
s3_path = build_s3_manifest_path(form_number)
s3_path = build_s3_manifest_path
local_path = download_manifest(temp_dir, s3_path)
write_and_upload_manifest(local_path) if config.include_manifest
ensure
Expand All @@ -83,8 +88,8 @@ def update_manifest
config.handle_error('Failed to update manifest', e)
end

def build_s3_manifest_path(form_number)
path = build_path(:file, s3_directory_path, "manifest_#{dated_directory_name(form_number)}", ext: '.csv')
def build_s3_manifest_path
path = build_path(:file, s3_directory_path, "manifest_#{container_directory}", ext: '.csv')
path.sub(%r{^/}, '')
end

Expand All @@ -105,7 +110,13 @@ def s3_uploader
end

def s3_directory_path
@s3_directory_path ||= build_path(:dir, parent_dir, upload_type.to_s, dated_directory_name(manifest_row[1]))
@s3_directory_path ||= build_path(:dir, parent_dir, upload_type.to_s, container_directory)
end

# /path/to/parent_dir/<UPLOAD_TYPE>/<SUBMISSION_DATE>-Form<FORM_NUMBER>
def container_directory
date = upload_type == :submission ? manifest_row[0] : Time.zone.today
dated_directory_name(manifest_row[1], date)
end

def generate_presigned_url(type: upload_type)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ def build!
config.handle_error("Failed building submission: #{id}", e)
end

def retrieval_data
extension = archive_type == :submission ? 'pdf' : 'zip'
final_path = "#{temp_directory_path}#{submission_file_name}.#{extension}"
[final_path, manifest_entry]
end

private

attr_reader :archive_type, :attachments, :config, :file_path, :id, :metadata, :pdf_already_exists, :submission,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@
allow(CarrierWave::SanitizedFile).to receive(:new).with(file_double).and_return(carrier_wave_file)
allow(CSV).to receive(:open).and_return(true)
allow(SimpleFormsApi::FormRemediation::SubmissionArchive).to(receive(:new).and_return(submission_archive_double))
allow(submission_archive_double).to receive(:build!).and_return([local_archive_path, manifest_entry])
allow(submission_archive_double).to receive_messages(
build!: [local_archive_path, manifest_entry],
retrieval_data: [local_archive_path, manifest_entry]
)
allow(SimpleFormsApi::FormRemediation::Uploader).to receive_messages(new: uploader)
allow(uploader).to receive(:get_s3_link).with(s3_archive_path).and_return('/s3_url/stuff.pdf')
allow(uploader).to receive_messages(get_s3_file: s3_file, store!: carrier_wave_file)
Expand All @@ -84,6 +87,11 @@
let(:type) { archive_type }

describe '.fetch_presigned_url' do
subject(:fetch_presigned_url) { described_class.fetch_presigned_url(benefits_intake_uuid, config:, type:) }

it 'returns the s3 link' do
expect(fetch_presigned_url).to eq('/s3_url/stuff.pdf')
end
end

describe '#initialize' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,27 @@
include_examples 'successfully built submission archive'
end
end

describe '#retrieval_data' do
subject(:retrieval_data) { archive_instance.retrieval_data }

let(:archive_instance) { described_class.new(**hydrated_submission_args) }
let(:file_name) { "#{temp_file_path}/#{submission_file_path}.#{type == :remediation ? 'zip' : 'pdf'}" }

it 'returns the correct archive path and manifest row' do
expect(retrieval_data.first).to include(file_name)
expect(retrieval_data.second).to eq(
[
submission.created_at,
form_type,
benefits_intake_uuid,
metadata['fileNumber'],
metadata['veteranFirstName'],
metadata['veteranLastName']
]
)
end
end
end
end
end
Loading