test(storage): isolate migrator spec service state

Restore ActiveStorage::Blob service globals after migrator specs so later attachment specs do not inherit leaked test doubles.

lib/active_storage/migrator.rb

@@ -0,0 +1,52 @@
+require 'yaml'
+require 'erb'
+
+class ActiveStorage::Migrator
+  def self.migrate(from_service_name, to_service_name, should_update_service_name: true)
+    configs = load_storage_config
+    # Check if services are configured correctly
+    if configs[from_service_name.to_s].nil? || configs[to_service_name.to_s].nil?
+      raise "Error: The services '#{from_service_name}' or '#{to_service_name}' are not configured correctly."
+    end
+
+    from_service = configure_service(from_service_name, configs)
+
+    configure_blob_service(from_service)
+
+    migrate_blobs(from_service_name, to_service_name, should_update_service_name: should_update_service_name)
+  end
+
+  def self.load_storage_config
+    yaml_with_env = ERB.new(File.read('config/storage.yml')).result
+    YAML.load(yaml_with_env)
+  end
+
+  def self.configure_blob_service(service)
+    ActiveStorage::Blob.service = service
+  end
+
+  def self.configure_service(service_name, configs = load_storage_config)
+    service_config = configs[service_name.to_s]
+    ActiveStorage::Service.configure(service_name, { service_name.to_sym => service_config })
+  end
+
+  def self.migrate_blobs(from_service_name, to_service_name, should_update_service_name: true)
+    to_service = configure_service(to_service_name)
+    blobs = ActiveStorage::Blob.where(service_name: from_service_name.to_s)
+    Rails.logger.debug { "#{blobs.count} Blobs to migrate from #{from_service_name} to #{to_service_name}" }
+
+    blobs.find_each do |blob|
+      Rails.logger.debug { '.' }
+
+      blob.open do |io|
+        checksum = blob.checksum
+        to_service.upload(blob.key, io, checksum: checksum)
+      end
+
+      blob.update!(service_name: to_service_name.to_s) if should_update_service_name
+    rescue ActiveStorage::FileNotFoundError => e
+      Rails.logger.warn { "Skipping missing blob #{blob.id} (#{blob.key}): #{e.message}" }
+    end
+    Rails.logger.debug { 'Successful migration' }
+  end
+end

lib/tasks/storage_migrations.rake

@@ -0,0 +1,13 @@
+namespace :storage do
+  desc 'Migrate blobs from one storage service to another'
+  # Example: FROM=local TO=amazon UPDATE_BLOB_SERVICE_NAME=true bundle exec rake storage:migrate
+  task migrate: :environment do
+    from_service = ENV.fetch('FROM', nil)
+    to_service = ENV.fetch('TO', nil)
+    should_update_service_name = ActiveModel::Type::Boolean.new.cast(ENV.fetch('UPDATE_BLOB_SERVICE_NAME', true))
+
+    raise 'Missing FROM or TO argument. Usage: FROM=service_name TO=service_name rake storage:migrate' if from_service.nil? || to_service.nil?
+
+    ActiveStorage::Migrator.migrate(from_service.to_sym, to_service.to_sym, should_update_service_name: should_update_service_name)
+  end
+end

spec/lib/active_storage/migrator_spec.rb

@@ -0,0 +1,175 @@
+require 'rails_helper'
+require 'fileutils'
+require 'stringio'
+require 'tmpdir'
+
+RSpec.describe ActiveStorage::Migrator do
+  around do |example|
+    original_services = ActiveStorage::Blob.services
+    original_service = ActiveStorage::Blob.service
+
+    example.run
+  ensure
+    ActiveStorage::Blob.services = original_services
+    ActiveStorage::Blob.service = original_service
+  end
+
+  describe '.migrate' do
+    let(:from_service_stub) { instance_double(ActiveStorage::Service, name: 'local') }
+    let(:to_service_stub) { instance_double(ActiveStorage::Service, name: 'amazon') }
+
+    before do
+      allow(ActiveStorage::Service).to receive(:configure).with('local', any_args).and_return(from_service_stub)
+      allow(ActiveStorage::Service).to receive(:configure).with('amazon', any_args).and_return(to_service_stub)
+    end
+
+    context 'when services are configured correctly' do
+      it 'migrates blobs from one service to another' do
+        expect(ActiveStorage::Service).to receive(:configure).with('local', any_args)
+        expect(described_class).to receive(:migrate_blobs).with('local', 'amazon', should_update_service_name: true)
+        expect { described_class.migrate('local', 'amazon') }.not_to raise_error
+      end
+
+      it 'passes the service-name update flag to blob migration' do
+        expect(described_class).to receive(:migrate_blobs).with('local', 'amazon', should_update_service_name: false)
+
+        described_class.migrate('local', 'amazon', should_update_service_name: false)
+      end
+
+      it 'does not override the application logger' do
+        allow(described_class).to receive(:migrate_blobs)
+
+        expect(Rails).not_to receive(:logger=)
+
+        described_class.migrate('local', 'amazon')
+      end
+    end
+
+    context 'when services are not configured correctly' do
+      it 'prints an error message' do
+        allow(ActiveStorage::Service).to receive(:configure).and_return(nil)
+        expect do
+          described_class.migrate('random', 'random')
+        end.to raise_error(RuntimeError, "Error: The services 'random' or 'random' are not configured correctly.")
+      end
+    end
+  end
+
+  describe '.migrate_blobs' do
+    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(described_class).to receive(:configure_service).with(:amazon).and_return(to_service)
+    end
+
+    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
+      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)
+
+      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 = 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')
+
+      from_service.delete(missing_blob.key)
+
+      expect do
+        described_class.migrate_blobs(:local, :amazon, should_update_service_name: true)
+      end.not_to raise_error
+
+      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
+      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 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

@@ -0,0 +1,64 @@
+require 'rake'
+require 'rails_helper'
+
+RSpec.describe Rake::Task do
+  describe 'storage_migrations' do
+    describe 'rake task' do
+      subject(:task) { described_class['storage:migrate'] }
+
+      before do
+        task.reenable
+      end
+
+      context 'when FROM argument is missing' do
+        it 'raises an error' do
+          with_modified_env FROM: nil, TO: 'amazon' do
+            expect do
+              task.invoke
+            end.to raise_error(RuntimeError,
+                               'Missing FROM or TO argument. Usage: FROM=service_name TO=service_name rake storage:migrate')
+          end
+        end
+      end
+
+      context 'when TO argument is missing' do
+        it 'raises an error' do
+          with_modified_env FROM: 'service_name', TO: nil do
+            expect do
+              task.invoke
+            end.to raise_error(RuntimeError,
+                               'Missing FROM or TO argument. Usage: FROM=service_name TO=service_name rake storage:migrate')
+          end
+        end
+      end
+
+      context 'when required arguments are present' do
+        it 'updates blob service names by default' do
+          expect(ActiveStorage::Migrator).to receive(:migrate).with(:local, :amazon, should_update_service_name: true)
+
+          with_modified_env FROM: 'local', TO: 'amazon', UPDATE_BLOB_SERVICE_NAME: nil do
+            task.invoke
+          end
+        end
+
+        it 'skips updating blob service names when disabled' do
+          expect(ActiveStorage::Migrator).to receive(:migrate).with(:local, :amazon, should_update_service_name: false)
+
+          with_modified_env FROM: 'local', TO: 'amazon', UPDATE_BLOB_SERVICE_NAME: 'false' 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
+end

spec/rails_helper.rb

@@ -7,6 +7,10 @@ require 'rspec/rails'
 require 'pundit/rspec'
 require 'sidekiq/testing'
 
+# Load Rake tasks
+require 'rake'
+Rails.application.load_tasks
+
 # test-prof helpers for tests optimization
 require 'test_prof/recipes/rspec/before_all'
 require 'test_prof/recipes/rspec/let_it_be'