Transcode to a temporary file which is only moved when finished

app/jobs/create_transcoded_item_job.rb

@@ -8,7 +8,7 @@ def perform(audio_file, codec_conversion)
     uuid = TranscodedItem.uuid_for(codec_conversion)
     path = TranscodedItem.path_for(codec_conversion, uuid)
     FileUtils.mkdir_p Pathname.new(path).parent
-    audio_file.convert(codec_conversion, path)
+    audio_file.convert_with_tmpfile(codec_conversion, path)
 
     TranscodedItem.transaction do
       # Check that the transcoded item was not created while we were executing

app/models/audio_file.rb

@@ -37,6 +37,14 @@ def check_self
     false
   end
 
+  def convert_with_tmpfile(codec_conversion, out_file_name)
+    tmp_path = File.join(Dir.tmpdir, "accentor_transcode-#{id}-#{codec_conversion.id}-#{SecureRandom.uuid}")
+    convert(codec_conversion, tmp_path)
+    FileUtils.mv tmp_path, out_file_name
+  ensure
+    FileUtils.rm_f tmp_path
+  end
+
   def convert(codec_conversion, out_file_name)
     parameters = codec_conversion.ffmpeg_params.split
     _stdout, status = Open3.capture2(

test/controllers/tracks_controller_test.rb

@@ -272,12 +272,11 @@ class TracksControllerTest < ActionDispatch::IntegrationTest
 class TracksControllerAudioTest < ActionDispatch::IntegrationTest
   setup do
     sign_in_as(create(:user))
-    AudioFile.alias_method :old_convert, :convert
-    AudioFile.define_method :convert, ->(_codec_conversion, out_file_name) { FileUtils.cp full_path, out_file_name }
+    install_audio_file_convert_stub
   end
 
   teardown do
-    AudioFile.alias_method :convert, :old_convert
+    uninstall_audio_file_convert_stub
   end
 
   test 'should return not_found if codec_conversion does not exit ' do

test/jobs/create_transcoded_item_job_test.rb

@@ -7,12 +7,11 @@ class CreateTranscodedItemJobTest < ActiveJob::TestCase
     @audio_file = create(:audio_file, location:, filename: '/base.flac', codec: flac)
     @codec_conversion = create(:codec_conversion)
 
-    AudioFile.alias_method :old_convert, :convert
-    AudioFile.define_method :convert, ->(_codec_conversion, out_file_name) { FileUtils.cp full_path, out_file_name }
+    install_audio_file_convert_stub
   end
 
   teardown do
-    AudioFile.alias_method :convert, :old_convert
+    uninstall_audio_file_convert_stub
   end
 
   test 'should create a new TranscodedItem with existing file' do
@@ -34,11 +33,14 @@ class CreateTranscodedItemJobTest < ActiveJob::TestCase
   test 'should abort when TranscodedItem was created while converting' do
     audio_file = @audio_file
     codec_conversion = @codec_conversion
-    AudioFile.define_method :convert, lambda { |_codec_conversion, _out_file_name|
+    AudioFile.define_method :convert, lambda { |_codec_conversion, out_file_name|
+      FileUtils.touch out_file_name
       TranscodedItem.create!(audio_file:, codec_conversion:, uuid: '0000-0000-0000')
     }
 
-    FileUtils.stubs(:rm_f).once
+    # Once during `convert_with_tmpfile`, once when noticing that the convert
+    # wasn't necessary anymore.
+    FileUtils.stubs(:rm_f).twice
 
     CreateTranscodedItemJob.perform_now(@audio_file, @codec_conversion)
   end

test/models/audio_file_test.rb

@@ -51,4 +51,30 @@ def setup
       @audio_file.convert(codec_conversion, TranscodedItem.path_for(codec_conversion, SecureRandom.uuid))
     end
   end
+
+  test 'convert_with_tmpfile should clean up after itself' do
+    codec_conversion = create(:codec_conversion)
+    path = TranscodedItem.path_for(codec_conversion, SecureRandom.uuid)
+    sent_filename = nil
+    FileUtils.stubs(:mv).once.with { |tmp_path, out_file_name| out_file_name == path && tmp_path == sent_filename }
+    with_stubbed_audio_file_convert ->(_codec_conversion, out_file_name) { sent_filename = out_file_name } do
+      @audio_file.convert_with_tmpfile(codec_conversion, path)
+    end
+
+    assert_not_equal sent_filename, path
+  end
+
+  test 'convert_with_tmpfile should clean up after itself even when raising' do
+    sent_filename = nil
+    FileUtils.stubs(:rm_f).once.with { |tmp_path| tmp_path == sent_filename }
+    with_stubbed_audio_file_convert lambda { |_codec_conversion, out_file_name|
+      sent_filename = out_file_name
+      raise AudioFile::FailedTranscode, 'oh no'
+    } do
+      codec_conversion = create(:codec_conversion)
+      assert_raises AudioFile::FailedTranscode, 'oh no' do
+        @audio_file.convert_with_tmpfile(codec_conversion, TranscodedItem.path_for(codec_conversion, SecureRandom.uuid))
+      end
+    end
+  end
 end

test/test_helper.rb

@@ -15,9 +15,28 @@
 require 'rails/test_help'
 require 'mocha/minitest'
 
+module AudioFileTestHelper
+  def install_audio_file_convert_stub(implementation = nil)
+    AudioFile.alias_method :old_convert, :convert
+    AudioFile.define_method :convert, implementation || ->(_codec_conversion, out_file_name) { FileUtils.cp full_path, out_file_name }
+  end
+
+  def uninstall_audio_file_convert_stub
+    AudioFile.alias_method :convert, :old_convert
+  end
+
+  def with_stubbed_audio_file_convert(implementation = nil)
+    install_audio_file_convert_stub implementation
+    yield
+  ensure
+    uninstall_audio_file_convert_stub
+  end
+end
+
 class ActiveSupport::TestCase
   include FactoryBot::Syntax::Methods
   include ActiveJob::TestHelper
+  include AudioFileTestHelper
 
   # Run tests in parallel with specified workers
   parallelize(workers: :number_of_processors)