fix(storage): update migrated blob service names
Default storage migrations to update active_storage_blobs.service_name after successful uploads while allowing copy-only runs with UPDATE_BLOB_SERVICE_NAME=false. Remove unused migrator helper methods and cover the migrator/rake flag behavior in specs.
This commit is contained in:
@@ -6,7 +6,7 @@ class ActiveStorage::Migrator
|
||||
Rails.logger = Logger.new($stdout)
|
||||
Rails.logger.level = Logger::DEBUG
|
||||
|
||||
def self.migrate(from_service_name, to_service_name)
|
||||
def self.migrate(from_service_name, to_service_name, 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?
|
||||
@@ -20,7 +20,7 @@ class ActiveStorage::Migrator
|
||||
|
||||
Rails.logger.debug { "#{ActiveStorage::Blob.count} Blobs to migrate from #{from_service_name} to #{to_service_name}" }
|
||||
|
||||
migrate_blobs(from_service, to_service)
|
||||
migrate_blobs(from_service, to_service, to_service_name, update_service_name: update_service_name)
|
||||
end
|
||||
|
||||
def self.load_storage_config
|
||||
@@ -28,22 +28,11 @@ class ActiveStorage::Migrator
|
||||
YAML.load(yaml_with_env)
|
||||
end
|
||||
|
||||
def self.configure_services(from_service_name, to_service_name, configs)
|
||||
from_service = ActiveStorage::Service.configure(from_service_name, { from_service_name.to_sym => configs[from_service_name.to_s] })
|
||||
to_service = ActiveStorage::Service.configure(to_service_name, { to_service_name.to_sym => configs[to_service_name.to_s] })
|
||||
[from_service, to_service]
|
||||
end
|
||||
|
||||
def self.configure_service(service_name, configs)
|
||||
service_config = configs[service_name.to_s]
|
||||
ActiveStorage::Service.configure(service_name, { service_name.to_sym => service_config })
|
||||
end
|
||||
|
||||
def self.configure_blob_service(service)
|
||||
ActiveStorage::Blob.service = service
|
||||
end
|
||||
|
||||
def self.migrate_blobs(_from_service, to_service)
|
||||
def self.migrate_blobs(_from_service, to_service, to_service_name, update_service_name: true)
|
||||
# Configure the blob service for the source service
|
||||
ActiveStorage::Blob.find_each do |blob|
|
||||
next unless blob.image?
|
||||
@@ -54,6 +43,8 @@ class ActiveStorage::Migrator
|
||||
checksum = blob.checksum
|
||||
to_service.upload(blob.key, io, checksum: checksum)
|
||||
end
|
||||
|
||||
blob.update!(service_name: to_service_name.to_s) if update_service_name
|
||||
end
|
||||
Rails.logger.debug { 'Successful migration' }
|
||||
end
|
||||
|
||||
@@ -3,9 +3,10 @@ namespace :storage do
|
||||
task migrate: :environment do
|
||||
from_service = ENV.fetch('FROM', nil)
|
||||
to_service = ENV.fetch('TO', nil)
|
||||
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)
|
||||
ActiveStorage::Migrator.migrate(from_service.to_sym, to_service.to_sym, update_service_name: update_service_name)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
require 'rails_helper'
|
||||
require 'stringio'
|
||||
|
||||
RSpec.describe ActiveStorage::Migrator do
|
||||
describe '.migrate' do
|
||||
@@ -14,9 +15,15 @@ RSpec.describe ActiveStorage::Migrator do
|
||||
it 'migrates blobs from one service to another' do
|
||||
expect(ActiveStorage::Service).to receive(:configure).with('local', any_args)
|
||||
expect(ActiveStorage::Service).to receive(:configure).with('amazon', any_args)
|
||||
expect(described_class).to receive(:migrate_blobs).with(from_service_stub, to_service_stub)
|
||||
expect(described_class).to receive(:migrate_blobs).with(from_service_stub, to_service_stub, 'amazon', 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(from_service_stub, to_service_stub, 'amazon', update_service_name: false)
|
||||
|
||||
described_class.migrate('local', 'amazon', update_service_name: false)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when services are not configured correctly' do
|
||||
@@ -28,4 +35,39 @@ RSpec.describe ActiveStorage::Migrator do
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '.migrate_blobs' do
|
||||
let(:from_service_stub) { instance_double(ActiveStorage::Service) }
|
||||
let(:to_service_stub) { instance_double(ActiveStorage::Service) }
|
||||
let(:blob) { instance_double(ActiveStorage::Blob, image?: true, key: 'blob-key', checksum: 'checksum') }
|
||||
let(:io) { StringIO.new('blob-content') }
|
||||
|
||||
before do
|
||||
allow(ActiveStorage::Blob).to receive(:find_each).and_yield(blob)
|
||||
allow(blob).to receive(:open).and_yield(io)
|
||||
end
|
||||
|
||||
it 'updates the blob service after uploading to the target service' 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
|
||||
|
||||
described_class.migrate_blobs(from_service_stub, to_service_stub, :amazon, 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(from_service_stub, to_service_stub, :amazon, update_service_name: false)
|
||||
end
|
||||
|
||||
it 'does not update the blob service when upload fails' do
|
||||
allow(to_service_stub).to receive(:upload).and_raise(ActiveStorage::IntegrityError)
|
||||
|
||||
expect(blob).not_to receive(:update!)
|
||||
expect do
|
||||
described_class.migrate_blobs(from_service_stub, to_service_stub, :amazon, update_service_name: true)
|
||||
end.to raise_error(ActiveStorage::IntegrityError)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -4,30 +4,49 @@ require 'rails_helper'
|
||||
RSpec.describe Rake::Task do
|
||||
describe 'storage_migrations' do
|
||||
describe 'rake task' do
|
||||
context 'when FROM argument is missing' do
|
||||
before do
|
||||
ENV['FROM'] = nil
|
||||
end
|
||||
subject(:task) { described_class['storage:migrate'] }
|
||||
|
||||
before do
|
||||
task.reenable
|
||||
end
|
||||
|
||||
context 'when FROM argument is missing' do
|
||||
it 'raises an error' do
|
||||
expect do
|
||||
described_class['storage:migrate'].invoke
|
||||
end.to raise_error(RuntimeError,
|
||||
'Missing FROM or TO argument. Usage: FROM=service_name TO=service_name rake storage:migrate')
|
||||
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
|
||||
before do
|
||||
ENV['FROM'] = 'service_name'
|
||||
ENV['TO'] = nil
|
||||
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, update_service_name: true)
|
||||
|
||||
with_modified_env FROM: 'local', TO: 'amazon', UPDATE_BLOB_SERVICE_NAME: nil do
|
||||
task.invoke
|
||||
end
|
||||
end
|
||||
|
||||
it 'raises an error' do
|
||||
expect do
|
||||
described_class['storage:migrate'].invoke
|
||||
end.to raise_error(RuntimeError,
|
||||
'Missing FROM or TO argument. Usage: FROM=service_name TO=service_name rake storage:migrate')
|
||||
it 'skips updating blob service names when disabled' do
|
||||
expect(ActiveStorage::Migrator).to receive(:migrate).with(:local, :amazon, update_service_name: false)
|
||||
|
||||
with_modified_env FROM: 'local', TO: 'amazon', UPDATE_BLOB_SERVICE_NAME: 'false' do
|
||||
task.invoke
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user