GitLab

Created by: teddywing

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