Skip to content

Commit efbf465

Browse files
authored
Remediated file paths and movement (#977)
* I think this should do what I want, but haven't tested it yet * Fixed uploader case matcher, adjusted main file path for remediated files * Adds some tests. Fixes some things up * Adds some more tests * Revert file structure generators to old setup. Use new EtdaUtilties functionality * Change file path generator for initial uploader to use remediated directory when needed * Fixed tests * use dependency injection for move_a_file. Adds tests for that method * Adds move_file test * Clean up code * Update etda_utilities * refactor * Moving files on publish and unpublish fixed to handle remediated files * Changed full_file_path tests to test current_location instead. This covers all the public methods for RemediatedFinalSubmissionFile * Adds tests for public methods in submission_release_service * Made these tests a little stronger and cleaned them up * Make file_class a required parameter for move_a_file (no default). Adds function to delete RemediatedFinalSubmissionFile when object is destroyed * Adds restricted_liberal_arts
1 parent 9dc81b0 commit efbf465

12 files changed

+387
-19
lines changed

Gemfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ gem 'font-awesome-rails', '~> 4.7.0.0'
2525
# Authentication gem
2626
gem "devise", ">= 4.7.1"
2727
# Shared libraries for workflow and explore
28-
gem 'etda_utilities', '~> 0.0'
28+
gem 'etda_utilities', "~> 0.22.0"
2929
# Ldap client
3030
gem 'net-ldap', '~> 0.16.1'
3131
# sftp for lionapth csv imports

Gemfile.lock

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ GEM
170170
rubocop (>= 1)
171171
smart_properties
172172
erubi (1.13.1)
173-
etda_utilities (0.21.0)
173+
etda_utilities (0.22.0)
174174
factory_bot (6.5.1)
175175
activesupport (>= 6.1.0)
176176
factory_bot_rails (6.4.4)
@@ -566,7 +566,7 @@ DEPENDENCIES
566566
down
567567
ed25519 (~> 1.2.4)
568568
enumerize (~> 2.6.0)
569-
etda_utilities (~> 0.0)
569+
etda_utilities (~> 0.22.0)
570570
factory_bot_rails (~> 6.4.0)
571571
faker (~> 3.5.1)
572572
font-awesome-rails (~> 4.7.0.0)

app/models/etda_file_paths.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ def explore_base_path
99
EXPLORE_BASE_PATH
1010
end
1111

12-
def move_a_file(fid, original_file_location)
12+
# file_class injection options: [FinalSubmissionFile, RemediatedFinalSubmissionFile]
13+
def move_a_file(fid, original_file_location, file_class:)
1314
error_msg = ''
1415

1516
unless File.exist?(original_file_location)
@@ -18,7 +19,7 @@ def move_a_file(fid, original_file_location)
1819
return error_msg
1920
end
2021

21-
updated_file = FinalSubmissionFile.find(fid)
22+
updated_file = file_class.find(fid)
2223
# this is calculating the new location based on updated submission and file attributes
2324

2425
new_location = updated_file.full_file_path

app/models/final_submission_file.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def move_file
5353

5454
path_builder = EtdaFilePaths.new
5555
original_file_location = "#{WORKFLOW_BASE_PATH}final_submission_files/#{path_builder.detailed_file_path(id)}#{asset_identifier}"
56-
path_builder.move_a_file(id, original_file_location)
56+
path_builder.move_a_file(id, original_file_location, file_class: self.class)
5757
end
5858

5959
def delete_file

app/models/remediated_final_submission_file.rb

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,47 @@ class RemediatedFinalSubmissionFile < ApplicationRecord
77
validates :submission_id, :asset, presence: true
88
validates :asset, virus_free: true
99

10+
after_save :move_file
11+
before_destroy :delete_file
12+
1013
def class_name
1114
self.class.to_s.underscore.dasherize
1215
end
16+
17+
def current_location
18+
# full file path including file name
19+
full_file_path + asset_identifier
20+
end
21+
22+
def full_file_path
23+
# file path w/o file name
24+
main_file_path + file_detail_path
25+
end
26+
27+
def file_detail_path
28+
# partial unique path built from file id -- ie('/01/01/')
29+
# Note: remediated is true to differentiate paths from original files
30+
EtdaFilePaths.new.detailed_file_path(id, remediated: true)
31+
end
32+
33+
def main_file_path
34+
# base portion of path up to file_detail_path
35+
SubmissionFilePath.new(submission).full_path_for_final_submissions.to_s
36+
end
37+
38+
private
39+
40+
def move_file
41+
return unless submission.status_behavior.released_for_publication?
42+
43+
path_builder = EtdaFilePaths.new
44+
original_file_location = "#{WORKFLOW_BASE_PATH}final_submission_files/#{path_builder.detailed_file_path(id, remediated: true)}#{asset_identifier}"
45+
path_builder.move_a_file(id, original_file_location, file_class: self.class)
46+
end
47+
48+
def delete_file
49+
return unless submission.status_behavior.released_for_publication?
50+
51+
FileUtils.rm current_location
52+
end
1353
end

app/services/submission_release_service.rb

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,28 @@ def unpublish(original_final_files)
3131
end
3232

3333
def final_files_for_submission(submission)
34-
# saves the file id and the original file path
34+
# saves the file id, the original file path, and the class name
3535
location_array = []
3636
submission.final_submission_files.each do |f|
37-
location_array << [f.id, f.current_location]
37+
location_array << [f.id, f.current_location, f.class.name]
38+
39+
# also include remediated final submission files to be moved
40+
next if f.remediated_final_submission_file.blank?
41+
42+
location_array << [f.remediated_final_submission_file.id,
43+
f.remediated_final_submission_file.current_location,
44+
f.remediated_final_submission_file.class.name]
3845
end
46+
3947
location_array
4048
end
4149

4250
def release_files(original_file_locations)
4351
etda_file_util = EtdaFilePaths.new
44-
original_file_locations.each do |fid, original_file_location|
45-
msg = etda_file_util.move_a_file(fid, original_file_location)
52+
original_file_locations.each do |fid, original_file_location, class_name|
53+
msg = etda_file_util.move_a_file(fid,
54+
original_file_location,
55+
file_class: class_name.constantize)
4656
next if msg.blank?
4757

4858
record_error(msg)
@@ -53,10 +63,10 @@ def release_files(original_file_locations)
5363

5464
def file_verification(original_files_array)
5565
file_error_list = []
56-
original_files_array.each do |id, original_file|
66+
original_files_array.each do |id, original_file, class_name|
5767
next if File.exist? original_file
5868

59-
err = "File Not Found for Final Submission File #{id}, #{original_file} "
69+
err = "File Not Found for #{class_name.underscore.humanize} #{id}, #{original_file} "
6070
record_error(err)
6171
file_error_list << err
6272
end

app/uploaders/submission_file_uploader.rb

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,21 +34,20 @@ def cache_dir
3434
end
3535

3636
def asset_prefix
37-
case model.class_name
38-
when 'final-submission-file'
37+
case model_class_name
38+
when 'final-submission-file', 'remediated-final-submission-file'
3939
Rails.root.join(WORKFLOW_BASE_PATH, 'final_submission_files')
4040
when 'admin-feedback-file'
4141
Rails.root.join(WORKFLOW_BASE_PATH, 'admin_feedback_files')
42-
when 'remediated_final_submission_files'
43-
Rails.root.join(WORKFLOW_BASE_PATH, 'remediated_final_submission_files')
4442
else
4543
Rails.root.join(WORKFLOW_BASE_PATH, 'format_review_files')
4644
end
4745
end
4846

4947
def asset_hash
5048
path_builder = EtdaFilePaths.new
51-
path_builder.detailed_file_path(model.id)
49+
path_builder.detailed_file_path(model.id,
50+
remediated: model_class_name == 'remediated-final-submission-file')
5251
end
5352

5453
def identity_subdir
@@ -65,4 +64,10 @@ def content_type_allowlist
6564
def extension_allowlist
6665
%w[pdf txt jpg jpeg png gif mp3 wav mov mp4 zip]
6766
end
67+
68+
private
69+
70+
def model_class_name
71+
model.class_name
72+
end
6873
end

spec/models/access_level_spec.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@
99

1010
describe '#ACCESS_LEVEL_KEYS' do
1111
it 'constant containing all access levels' do
12-
expect(described_class::ACCESS_LEVEL_KEYS).to contain_exactly('open_access', 'restricted_to_institution', 'restricted', '')
12+
expect(described_class::ACCESS_LEVEL_KEYS).to contain_exactly('open_access', 'restricted_to_institution', 'restricted_liberal_arts', 'restricted', '')
1313
expect(described_class::ACCESS_LEVEL_KEYS).to include('open_access')
1414
expect(described_class::ACCESS_LEVEL_KEYS).to include('restricted')
1515
expect(described_class::ACCESS_LEVEL_KEYS).to include('restricted_to_institution')
16+
expect(described_class::ACCESS_LEVEL_KEYS).to include('restricted_liberal_arts')
1617
expect(described_class::ACCESS_LEVEL_KEYS).to include('')
17-
expect(described_class::ACCESS_LEVEL_KEYS.length).to eq(4)
18+
expect(described_class::ACCESS_LEVEL_KEYS.length).to eq(5)
1819
end
1920
end
2021

spec/models/etda_file_paths_spec.rb

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,61 @@
4141
expect(described_class.new.explore_open).to eql("#{explore_path}open_access/")
4242
end
4343
end
44+
45+
describe '#move_a_file' do
46+
let(:file_paths) { described_class.new }
47+
let(:fid) { 123 }
48+
let(:original_file_location) { '/tmp/original/file.pdf' }
49+
let(:updated_file) do
50+
instance_double('FinalSubmissionFile',
51+
full_file_path: '/dest/path/',
52+
asset_identifier: 'file.pdf')
53+
end
54+
55+
context 'when the original file does not exist' do
56+
it 'returns an error message and does not look up the file' do
57+
allow(File).to receive(:exist?).with(original_file_location).and_return(false)
58+
allow(Rails.logger).to receive(:error)
59+
file_class = class_double('FinalSubmissionFile')
60+
allow(file_class).to receive(:find)
61+
62+
result = file_paths.move_a_file(fid, original_file_location, file_class: file_class)
63+
64+
expect(result).to eq("File not found: #{original_file_location}")
65+
expect(file_class).not_to have_received(:find)
66+
end
67+
end
68+
69+
context 'when the original file exists' do
70+
it 'moves the file using the provided file_class' do
71+
file_class = class_double('RemediatedFinalSubmissionFile').as_stubbed_const
72+
73+
allow(File).to receive(:exist?).with(original_file_location).and_return(true)
74+
allow(file_class).to receive(:find).with(fid).and_return(updated_file)
75+
allow(FileUtils).to receive(:mkpath)
76+
allow(FileUtils).to receive(:mv)
77+
78+
result = file_paths.move_a_file(fid, original_file_location, file_class: file_class)
79+
80+
expect(FileUtils).to have_received(:mkpath).with('/dest/path/')
81+
expect(FileUtils).to have_received(:mv).with(original_file_location, '/dest/path/file.pdf')
82+
expect(result).to eq('')
83+
end
84+
end
85+
86+
context 'when using FinalSubmissionFile as the file_class' do
87+
it 'uses FinalSubmissionFile to find the record' do
88+
file_class = class_double('FinalSubmissionFile').as_stubbed_const
89+
90+
allow(File).to receive(:exist?).with(original_file_location).and_return(true)
91+
allow(file_class).to receive(:find).with(fid).and_return(updated_file)
92+
allow(FileUtils).to receive(:mkpath)
93+
allow(FileUtils).to receive(:mv)
94+
95+
file_paths.move_a_file(fid, original_file_location, file_class: file_class)
96+
97+
expect(file_class).to have_received(:find).with(fid)
98+
end
99+
end
100+
end
44101
end

spec/models/remediated_final_submission_file_spec.rb

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,38 @@
2222
expect(described_class.new.class_name).to eql('remediated-final-submission-file')
2323
end
2424

25+
describe '#current_location' do
26+
context 'when submission is waiting for publication release' do
27+
it 'returns full workflow file path w filename' do
28+
submission = FactoryBot.create :submission, :waiting_for_publication_release
29+
remediated_final_submission_file = FactoryBot.create(:remediated_final_submission_file,
30+
submission_id: submission.id)
31+
remediated_final_submission_file.id = 1234
32+
expect(remediated_final_submission_file.current_location)
33+
.to eq(
34+
"#{WORKFLOW_BASE_PATH}final_submission_files/" \
35+
"#{EtdaFilePaths.new.detailed_file_path(remediated_final_submission_file.id, remediated: true)}" \
36+
"#{remediated_final_submission_file.asset_identifier}"
37+
)
38+
end
39+
end
40+
41+
context 'when submission has been released for publication' do
42+
it 'returns full explore file path w filename' do
43+
submission = FactoryBot.create :submission, :released_for_publication
44+
remediated_final_submission_file = FactoryBot.create(:remediated_final_submission_file,
45+
submission_id: submission.id)
46+
remediated_final_submission_file.id = 1234
47+
expect(remediated_final_submission_file.current_location)
48+
.to eq(
49+
"#{EXPLORE_BASE_PATH + submission.access_level_key}" \
50+
"/#{EtdaFilePaths.new.detailed_file_path(remediated_final_submission_file.id, remediated: true)}" \
51+
"#{remediated_final_submission_file.asset_identifier}"
52+
)
53+
end
54+
end
55+
end
56+
2557
describe 'virus scanning' do
2658
# We have these Virus scanner tests for both format review file and final_submission_file models
2759
# Not sure if it's valauble to have them here as well?
@@ -60,4 +92,55 @@
6092
end
6193
end
6294
end
95+
96+
describe 'after_save :move_file' do
97+
let(:submission) do
98+
FactoryBot.create :submission,
99+
:released_for_publication
100+
end
101+
102+
let(:remediated_file) do
103+
FactoryBot.create :remediated_final_submission_file,
104+
submission: submission
105+
end
106+
107+
it 'calls EtdaFilePaths.move_a_file with remediated_file: true' do
108+
path_builder = instance_double(EtdaFilePaths)
109+
allow(EtdaFilePaths).to receive(:new).and_return(path_builder)
110+
allow(path_builder).to receive(:detailed_file_path).and_return('path/to/file/')
111+
allow(path_builder).to receive(:move_a_file)
112+
113+
original_file_location = "#{WORKFLOW_BASE_PATH}final_submission_files/" \
114+
"#{path_builder.detailed_file_path(remediated_file.id, remediated: true)}" \
115+
"#{remediated_file.asset_identifier}"
116+
117+
remediated_file.save!
118+
119+
# Expect twice: once during create, once during save!
120+
expect(path_builder)
121+
.to have_received(:move_a_file)
122+
.with(remediated_file.id, original_file_location, file_class: remediated_file.class)
123+
.twice
124+
end
125+
end
126+
127+
describe 'before_destroy :delete_file' do
128+
let(:submission) do
129+
FactoryBot.create :submission,
130+
:released_for_publication
131+
end
132+
133+
let(:remediated_file) do
134+
FactoryBot.create :remediated_final_submission_file,
135+
submission: submission
136+
end
137+
138+
it 'deletes the file from the filesystem' do
139+
expect(File.exist?(remediated_file.current_location)).to be true
140+
141+
remediated_file.destroy
142+
143+
expect(File.exist?(remediated_file.current_location)).to be false
144+
end
145+
end
63146
end

0 commit comments

Comments
 (0)