最近は担当製品の関連アプリを改修しているのですが、Railsのバージョンを5.2系から6.0にアップグレードしました。そのときにActiveStorageでハマったので備忘録を残しておきます。なお、担当製品は既にRails6にしてあります。ただし、まだActiveStorage使ってない。その時の記事はこちら。
普通にRails6へのアップグレード
先ほどの記事を読みながらどういう対応をしたかを思い出して対応。一通り、手動で動かしてみたところ、問題なさそう。そして、RSpecを実行するとめちゃくちゃ落ちる！！びっくりするくらい落ちる！！
テストが落ちる原因を探る
テストが落ちた原因は、まぁRailsのバージョンを上げたのだから、バージョンの差異が原因でしょう。ActiveStorageが絡んでいるところが落ちていたので、調査しました。Qiitaに変更点がまとめられている記事がありました。ありがたい。
ここの中で「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| # => Rails6だと、ここでopenできずに落ちる！ # 検証する end end end def check_files self.errors.add(:files, :invalid) unless valid_files? end end
ActiveStorage::Blobモデルが保存されていないため、ActiveStorage::FileNotFoundErrorで落ちてました。
アップロード前にファイルの内容を検証できるのか？
blobをopenしても落ちるので、何かいい方法はないものかと調べましたら、Railsのissueに有力な情報がありました。
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 # テストにてattachで添付時はこちら 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
とりあえず、これでモデルのテストでもダイレクトアップロードを考慮したテストを行うことができました。
