WorkbenchImportWorker: Use temp files for entry group subdir Zips

Instead of creating our own files for these subdirectory Zip files, use
temp files.

There are two advantages to doing this:

1. We can customise the location they get saved to. This means we can
   use `Rails.root` as a default, but specify a custom location in a
   Rails config variable.
2. The files will be automatically deleted on every garbage collection.

In actuality, #2 above was already handled by the `eg_file.unlink` line,
and #1 could be easily added to the existing code.

Using a `Tempfile`, though, makes the fact that these are temporary
files more explicit.

The reason why we want to be able to customise the location of these
tempfiles is that in our production environment set up by Puppet, the
`#{Rails.root}/tmp/imports` directory isn't writable by the application,
which causes this code to error. We thus want a way to change the path
in different environments.

We keep the `eg_file.unlink` call here because we want the temporary
file to be deleted as soon as possible. The Ruby documentation even
recommends it
(https://ruby-doc.org/stdlib-2.5.0/libdoc/tempfile/rdoc/Tempfile.html#class-Tempfile-label-Explicit+close):

> it's good practice to do so: not explicitly deleting unused Tempfiles
> can potentially leave behind large amounts of tempfiles on
> the filesystem until they're garbage collected.

I got rid of the `#upload_entry_group_tmpfile` method in this change,
moving its contents to `#upload_entry_group_stream`. This was easy
enough to do since it was only called in that one place. The reason I
did this is because it took an open `File` object as an argument and
closed the file at the end of the method. That's a major side effect. I
don't think the method should be closing a file it doesn't own. After
moving this work to `#upload_entry_group_stream`, we can close and
unlink the file in the same place where it was created.

Refs #4315

1 files changed, 14 insertions, 8 deletions

diff --git a/app/workers/workbench_import_worker.rb b/app/workers/workbench_import_worker.rb
index 6420be835..13b0efae0 100644
--- a/app/workers/workbench_import_worker.rb
+++ b/app/workers/workbench_import_worker.rb
@@ -53,17 +53,18 @@ class WorkbenchImportWorker
   end
 
   def upload_entry_group_stream eg_name, eg_stream
-    FileUtils.mkdir_p(Rails.root.join('tmp', 'imports'))
-
-    File.open(Rails.root.join('tmp', 'imports', "WorkbenchImport_#{eg_name}_#{$$}.zip"), 'wb') do |file|
+    eg_file_path = Tempfile.open(
+      ["WorkbenchImport_#{eg_name}_", '.zip'],
+      temp_directory
+    ) do |f|
       eg_stream.rewind
-      file.write eg_stream.read
+      f.write eg_stream.read
+
+      f.path
     end
 
-    upload_entry_group_tmpfile eg_name, File.new(Rails.root.join('tmp', 'imports', "WorkbenchImport_#{eg_name}_#{$$}.zip"))
-  end
-    
-  def upload_entry_group_tmpfile eg_name, eg_file
+    eg_file = File.open(eg_file_path)
+
     result = execute_post eg_name, eg_file
     if result && result.status < 400
       @entries += 1
@@ -117,6 +118,11 @@ class WorkbenchImportWorker
         file: HTTPService.upload(file, 'application/zip', "#{name}.zip") } }
   end
 
+  def temp_directory
+    Rails.application.config.try(:import_temporary_directory) ||
+      Rails.root.join('tmp', 'imports')
+  end
+
   # Lazy Values
   # ===========