最近は担当製品の関連アプリを改修しているのですが、Railsのバージョンを5.2系から6.0にアップグレードしました。そのときにActiveStorageでハマったので備忘録を残しておきます。なお、担当製品は既にRails6にしてあります。ただし、まだActiveStorage使ってない。その時の記事はこちら。
patorash.hatenablog.com
普通にRails6へのアップグレード
先ほどの記事を読みながらどういう対応をしたかを思い出して対応。一通り、手動で動かしてみたところ、問題なさそう。そして、RSpecを実行するとめちゃくちゃ落ちる！！びっくりするくらい落ちる！！
テストが落ちる原因を探る
テストが落ちた原因は、まぁRailsのバージョンを上げたのだから、バージョンの差異が原因でしょう。ActiveStorageが絡んでいるところが落ちていたので、調査しました。Qiitaに変更点がまとめられている記事がありました。ありがたい。
qiita.com
ここの中で「3. アップロードしたファイルがストレージに保存されるタイミングがsave時に変更」という見出しが…。
Rails5.2までは、インスタンスのattributeに代入した時点で、アップロードしたファイルがストレージに保存されていました。
Rails6.0では、saveしたタイミングでストレージに保存されるよう変更になっています。saveトランザクションがコミットされた後に、ストレージに保存されます。
おぉ…、まさにこれが原因でした…。
なぜsave後にアップロードでテストが落ちるのか？
理由は、バリデーションのためにActiveStorageにアップロードしたファイルの中身を参照して検証していたからです。
class Message < ApplicationRecord
has_many_attached :files
validates :files, presence: true
validate :check_files, if: -> { self.files.attached? }
private
def valid_files?
self.files.all? do |file|
file.blob.open do |f|
end
end
end
def check_files
self.errors.add(:files, :invalid) unless valid_files?
end
end
ActiveStorage::Blobモデルが保存されていないため、ActiveStorage::FileNotFoundErrorで落ちてました。
アップロード前にファイルの内容を検証できるのか？
blobをopenしても落ちるので、何かいい方法はないものかと調べましたら、Railsのissueに有力な情報がありました。
github.com
attachment_changes['files'].attachablesを使え、とあります。（has_many_attachedならattachablesで、has_one_attachedならattachableになる）
pryを使いながら検証してみたところ、attachableの中身は、{ io: Fileオブジェクト, filename: 'ファイル名' }というHashでした。
そこで、以下のように修正。
def valid_files?
self.attachment_changes['files'].attachables.pluck(:io).all? do |file|
end
end
これでRSpecを実行したところ…。modelのテストは通りました！修正成功です！
SystemSpecが落ちる…
しかし、テストを全部流してみたらSystemSpecが落ちました。手動で画面を操作してみたところ、確かに落ちます。しかも、先ほどの変更したところで…。
self.attachment_changes['files'].attachablesの中身がHashの配列ではなく、Stringの配列になっていました。なんでやねん…。
そもそも最初は手動テストで通ってのに、RSpecのテストが落ちるから修正したら、手動テストが通らなくなったので、もしかしたら画面からの場合はモデルの保存前でもActiveStorageにファイルがアップロードされてる？と考えてpryを仕込みながら調査したところ、ビンゴ。
2020-11-21 追記
もしかしたら画面からの場合はモデルの保存前でもActiveStorageにファイルがアップロードされてる？
これは、ActiveStorageのダイレクトアップロードしているからでした…。それなら当然ですね…。
画面からファイルをアップロードした場合
Messageモデルを保存していない状態で、blobオブジェクトの中身を見た結果。idがあるので、保存済みの証拠です。つまり、ActiveStorageにアプロード済み。（2020-11-21 追記：ダイレクトアップロードしていたからです！）
[1] pry(#<Message>)> self.files.first.blob
=> #<ActiveStorage::Blob:0x00007fb56b9479b0
id: 320,
key: "************************",
filename: "test.csv",
content_type: "text/csv",
metadata: {},
byte_size: 1452,
checksum: "**************************",
created_at: Fri, 20 Nov 2020 13:45:17 JST +09:00>
RSpecでファイルをattachした場合
テストでは、attachメソッドで付けてます。
message = FactoryBot.build(:message)
file = File.open(Rails.root.join('spec', 'files', 'test.csv'), 'r')
message.files.attach(io: file, filename: File.basename(file))
pryで中身を見た結果は、以下の通り。idがnilのため、ActiveStorageにはまだアップロードされていません。
[1] pry(#<Message>)> self.files.first.blob
=> #<ActiveStorage::Blob:0x00007fdcfd305d68
id: nil,
key: nil,
filename: "test.csv",
content_type: "text/csv",
metadata: {"identified"=>true},
byte_size: 600,
checksum: "**************************",
created_at: nil>
導かれる仮説は…
2020-11-21 追記
ダイレクトアップロードしていたからだったので、この仮説は間違っていました。すみません。
- 画面を経由せずにActiveStorageにアップロードする場合は、モデルの保存が成功したタイミングでアップロード。
- 画面を経由してActiveStorageにアップロードする場合は、モデルの保存をする以前にアップロード。（これが間違い。ダイレクトアップロードしてたから）
となると、画面を経由しているケースを想定した処理と、経由していないケースを想定した処理を書かないといけません…。ここにすごく違和感を感じる…。
修正
とりあえず、コードは以下のようになりました。
class Message < ApplicationRecord
has_many_attached :files
validates :files, presence: true
validate :check_files, if: -> { self.files.attached? }
private
def valid_files?
if uploaded?
self.files.all? do |file|
file.blob.open do |f|
end
end
else
self.attachment_changes['files'].attachables.pluck(:io).all? do |file|
end
end
end
def check_files
self.errors.add(:files, :invalid) unless valid_files?
end
def uploaded?
self.files.all? { |file| file.blob.persisted? }
end
end
これでテストは全て通るようになりました。やった！！！
不満点
違和感を感じると書いていますが、その違和感の正体は、「これだとモデルのユニットテストでActiveStorageにファイルが保存されているケースを検証できない」という点です。attachだと保存されないから…。画面からのアップロードでも、ActiveStorageのアップロード前の状態で、デフォルトで一時ファイルへのパス情報をどっかに保存していたらいいのに…。ダイレクトアップロードでなければ一時ファイルに保存されていることを確認したので、私が間違えていました。
何か他にいい方法をご存知の方がいらっしゃいましたらコメントでもツイッターでもいいので教えてください！！
2020-11-21 追記
画面からアップロードした際に先にクラウドにアップロードされていた原因はダイレクトアップロードをしていたためでした。先にコード読めよって話ですけど、自分が作ったところじゃなかったのでテストが落ちる原因にばかり考えが巡ってしまってActiveStorageのダイレクトアップロードの存在を忘れてました。このブログを書いた後に、急にピーン！ときました。
ダイレクトアップロードではない場合、当然ながらモデルが保存されるまではアップロードされず、一時ファイルに置かれていました。attachableの内容もHashではなく、ActionDispatch::Http::UploadedFileになっていたので、これに合わせて修正すればよさそうです。
>> self.attachment_changes['files'].attachables.first
=> #<ActionDispatch::Http::UploadedFile:0x00007f9d4a54ef18
@tempfile=#<Tempfile:/var/folders/7m/9fg43zzx58lb4715wm1dkj00yj5swh/T/RackMultipart20201121-93972-xgi34u.csv>,
@original_filename="test.csv",
@content_type="text/csv",
@headers="Content-Disposition: form-data; name=\"message[files][]\"; filename=\"test.csv\"\r\nContent-Type: text/csv\r\n">
となると………
- ダイレクトアップロードの場合のバリデーション
- ダイレクトアップロードじゃない場合のバリデーション
- attachメソッドを使った場合のバリデーション
の3パターンを考えないといけないわけか…。うっ、頭が…。
（これはあくまでファイルの中身をチェックして保存したい場合の話なので、普通に画像をアップロードするだけ、とかだとこんな面倒な話にはなりませんのでActiveStorageが怖いわけではありません）
2020-11-26 追記
ダイレクトアップロードじゃないケースは実装上、ないので、そこはテストで放置して、一応attachメソッドも考慮したテストを追加して対応しました。
FactoryBotのコードは、こう。
FactoryBot.define do
factory :message do
sequence(:content) { |n| "メッセージ#{n}" }
end
trait :with_direct_upload_file do
after :build do |message|
valid_file = File.open(Rails.root.join('spec', 'files', 'valid_file.csv'), 'r')
blob = ActiveStorage::Blob.create_and_upload!(
io: valid_file,
filename: File.basename(valid_file),
identify: false
)
message.files.build(blob: blob)
end
end
trait :with_attach_file do
after :build do |message|
valid_file = File.open(Rails.root.join('spec', 'files', 'valid_file.csv'), 'r')
message.files.attach(io: valid_file, filename: File.basename(valid_file))
end
end
end
そして、RSpecはこう。
RSpec.describe Message, type: :model do
context 'ダイレクトアップロードの場合' do
subject { FactoryBot.build(:message, :with_direct_upload_file) }
it '保存できること' do
end
end
context 'attachの場合' do
subject { FactoryBot.build(:message, :with_attach_file) }
it '保存できること' do
end
end
end
とりあえず、これでモデルのテストでもダイレクトアップロードを考慮したテストを行うことができました。
