test(storage): cover blob migration contract

Replace mock-only migrator specs with disk-backed ActiveStorage blob coverage for non-image uploads, service-name updates, missing files, upload failures, and idempotent reruns.

Add a rake task regression for re-enabled repeated invocation.

spec/lib/active_storage/migrator_spec.rb

@@ -1,5 +1,7 @@
 require 'rails_helper'
+require 'fileutils'
 require 'stringio'
+require 'tmpdir'
 
 RSpec.describe ActiveStorage::Migrator do
   describe '.migrate' do
@@ -44,61 +46,120 @@ RSpec.describe ActiveStorage::Migrator do
   end
 
   describe '.migrate_blobs' do
-    let(:to_service_stub) { instance_double(ActiveStorage::Service) }
-    let(:source_blobs) { instance_double(ActiveRecord::Relation, count: 1) }
-    let(:blob) { instance_double(ActiveStorage::Blob, id: 1, key: 'blob-key', checksum: 'checksum') }
-    let(:io) { StringIO.new('blob-content') }
+    let(:from_root) { Dir.mktmpdir('active-storage-local') }
+    let(:to_root) { Dir.mktmpdir('active-storage-amazon') }
+    let(:other_root) { Dir.mktmpdir('active-storage-other') }
+    let(:from_service) { build_disk_service('local', from_root) }
+    let(:to_service) { build_disk_service('amazon', to_root) }
+    let(:other_service) { build_disk_service('other', other_root) }
 
     before do
-      allow(ActiveStorage::Service).to receive(:configure).with(:amazon, any_args).and_return(to_service_stub)
-      allow(ActiveStorage::Blob).to receive(:where).with(service_name: 'local').and_return(source_blobs)
-      allow(source_blobs).to receive(:find_each).and_yield(blob)
-      allow(blob).to receive(:open).and_yield(io)
-      allow(blob).to receive(:update!)
-      allow(to_service_stub).to receive(:upload)
+      allow(described_class).to receive(:configure_service).with(:amazon).and_return(to_service)
     end
 
-    it 'migrates blobs regardless of content type' do
-      expect(to_service_stub).to receive(:upload).with('blob-key', io, checksum: 'checksum').ordered
-      expect(blob).to receive(:update!).with(service_name: 'amazon').ordered
+    around do |example|
+      original_services = ActiveStorage::Blob.services
+      original_service = ActiveStorage::Blob.service
+
+      ActiveStorage::Blob.services = {
+        'local' => from_service,
+        'amazon' => to_service,
+        'other' => other_service
+      }
+      ActiveStorage::Blob.service = from_service
+
+      example.run
+    ensure
+      ActiveStorage::Blob.services = original_services
+      ActiveStorage::Blob.service = original_service
+      FileUtils.rm_rf([from_root, to_root, other_root])
+    end
+
+    it 'uploads non-image blobs and updates their service name' do
+      blob = create_blob(service_name: 'local', filename: 'invoice.pdf', content_type: 'application/pdf', content: 'pdf-content')
 
       described_class.migrate_blobs(:local, :amazon, should_update_service_name: true)
+
+      expect(blob).not_to be_image
+      expect(blob.reload.service_name).to eq('amazon')
+      expect(to_service.download(blob.key)).to eq('pdf-content')
     end
 
     it 'only migrates blobs from the source service' do
-      expect(ActiveStorage::Blob).to receive(:where).with(service_name: 'local').and_return(source_blobs)
-      expect(source_blobs).to receive(:find_each).and_yield(blob)
+      local_blob = create_blob(service_name: 'local', filename: 'audio.mp3', content_type: 'audio/mpeg', content: 'audio-content')
+      other_blob = create_blob(service_name: 'other', filename: 'other.pdf', content_type: 'application/pdf', content: 'other-content')
 
       described_class.migrate_blobs(:local, :amazon, should_update_service_name: true)
-    end
 
-    it 'skips the blob service update when the flag is disabled' do
-      expect(to_service_stub).to receive(:upload).with('blob-key', io, checksum: 'checksum')
-      expect(blob).not_to receive(:update!)
-
-      described_class.migrate_blobs(:local, :amazon, should_update_service_name: false)
+      expect(local_blob.reload.service_name).to eq('amazon')
+      expect(to_service.download(local_blob.key)).to eq('audio-content')
+      expect(other_blob.reload.service_name).to eq('other')
+      expect(to_service.exist?(other_blob.key)).to be(false)
+      expect(other_service.download(other_blob.key)).to eq('other-content')
     end
 
     it 'skips missing source files and continues migration' do
-      missing_blob = instance_double(ActiveStorage::Blob, id: 2, key: 'missing-key')
+      missing_blob = create_blob(service_name: 'local', filename: 'missing.pdf', content_type: 'application/pdf', content: 'missing-content')
+      existing_blob = create_blob(service_name: 'local', filename: 'existing.pdf', content_type: 'application/pdf', content: 'existing-content')
 
-      allow(source_blobs).to receive(:find_each).and_yield(missing_blob).and_yield(blob)
-      allow(missing_blob).to receive(:open).and_raise(ActiveStorage::FileNotFoundError)
+      from_service.delete(missing_blob.key)
 
-      expect(missing_blob).not_to receive(:update!)
-      expect(to_service_stub).to receive(:upload).with('blob-key', io, checksum: 'checksum')
-      expect(blob).to receive(:update!).with(service_name: 'amazon')
+      expect do
+        described_class.migrate_blobs(:local, :amazon, should_update_service_name: true)
+      end.not_to raise_error
 
-      described_class.migrate_blobs(:local, :amazon, should_update_service_name: true)
+      expect(missing_blob.reload.service_name).to eq('local')
+      expect(to_service.exist?(missing_blob.key)).to be(false)
+      expect(existing_blob.reload.service_name).to eq('amazon')
+      expect(to_service.download(existing_blob.key)).to eq('existing-content')
     end
 
     it 'does not update the blob service when upload fails' do
-      allow(to_service_stub).to receive(:upload).and_raise(ActiveStorage::IntegrityError)
+      blob = create_blob(service_name: 'local', filename: 'report.pdf', content_type: 'application/pdf', content: 'report-content')
+
+      allow(to_service).to receive(:upload).and_raise(ActiveStorage::IntegrityError)
 
-      expect(blob).not_to receive(:update!)
       expect do
         described_class.migrate_blobs(:local, :amazon, should_update_service_name: true)
       end.to raise_error(ActiveStorage::IntegrityError)
+
+      expect(blob.reload.service_name).to eq('local')
+      expect(to_service.exist?(blob.key)).to be(false)
+    end
+
+    it 'skips the blob service update when the flag is disabled' do
+      blob = create_blob(service_name: 'local', filename: 'manual.pdf', content_type: 'application/pdf', content: 'manual-content')
+
+      described_class.migrate_blobs(:local, :amazon, should_update_service_name: false)
+
+      expect(blob.reload.service_name).to eq('local')
+      expect(to_service.download(blob.key)).to eq('manual-content')
+    end
+
+    it 'does not rewrite blobs that were already migrated' do
+      blob = create_blob(service_name: 'local', filename: 'archive.pdf', content_type: 'application/pdf', content: 'archive-content')
+
+      allow(to_service).to receive(:upload).and_call_original
+
+      2.times { described_class.migrate_blobs(:local, :amazon, should_update_service_name: true) }
+
+      expect(to_service).to have_received(:upload).with(blob.key, an_instance_of(Tempfile), checksum: blob.checksum).once
+      expect(blob.reload.service_name).to eq('amazon')
+      expect(ActiveStorage::Blob.where(id: blob.id, service_name: 'local')).to be_empty
+    end
+
+    def build_disk_service(service_name, root)
+      ActiveStorage::Service.configure(service_name, { service_name => { service: 'Disk', root: root } })
+    end
+
+    def create_blob(service_name:, filename:, content_type:, content:)
+      ActiveStorage::Blob.create_and_upload!(
+        io: StringIO.new(content),
+        filename: filename,
+        content_type: content_type,
+        service_name: service_name,
+        identify: false
+      )
     end
   end
 end

spec/lib/tasks/rake/task_storage_migrations_spec.rb

@@ -48,6 +48,16 @@ RSpec.describe Rake::Task do
             task.invoke
           end
         end
+
+        it 'can be invoked again after the task is re-enabled' do
+          expect(ActiveStorage::Migrator).to receive(:migrate).twice.with(:local, :amazon, should_update_service_name: true)
+
+          with_modified_env FROM: 'local', TO: 'amazon', UPDATE_BLOB_SERVICE_NAME: nil do
+            task.invoke
+            task.reenable
+            task.invoke
+          end
+        end
       end
     end
   end