こんにちは、エンジニアのさもです。
CarrierWaveを使った画像アップロードが実装済みの画面を改修(追加で画像をアップロードできるようにする修正)しているときに、なぜかたまに参照している画像が存在しないという旨のエラーで落ちていました。
自分のうっかりミスだったのですが、ちゃんと動くときもあり、陥りやすいミスなのでメモしておきます。
スポンサーリンク
CarrierWave自体の説明は省きます。
問題のコードはこちらでした。
class ImageUploader < CarrierWave::Uploader::Base if Rails.env.development? || Rails.env.test? # ローカル storage :file else # S3 storage :fog end def cache_dir if Rails.env.development? || Rails.env.test? "#{Rails.root}/public/tmp_images" else "/uploads/tmp_images" end end def store_dir "#{Image::UPLOAD_PATH}/#{model.id}" end # 許可する画像の拡張子 def extension_white_list %w(jpg jpeg gif png) end # 変換したファイルのファイル名の規則 def filename if original_filename.present? name = "#{Time.zone.now.strftime('%Y%m%d_%H%M%S')}.#{file.extension}" name.downcase else original_filename end end
よくありがちなuploaderですが、ここにアンチパターンがありました。
name = "#{Time.zone.now.strftime('%Y%m%d_%H%M%S')}.#{file.extension}"
これですね。時間をファイル名にしている部分です。
filenameは、アップロードして、登録されるまでに複数回呼ばれます(ファイル名をつけるときと、カラムに値を入れるとき?)
ですが、1回目と2回目の間に時間差があると、ファイル名が変わってしまい、参照するファイル名と実際のファイル名が変わってしまうのです。
時間以外にも、呼ぶたびにファイル名が変わる実装は非推奨です。改修前は、処理が早かったので、時間差が生まれにくく、なんとか上手くいっていたのですね。
ではどうすればいいかですが、公式に書き方が書いてありました。
def filename "#{secure_token}.#{file.extension}" if original_filename.present? end protected def secure_token var = :"@#{mounted_as}_secure_token" model.instance_variable_get(var) or model.instance_variable_set(var, SecureRandom.uuid) end
ポイントは、以下の2点です。
- SecureRandom.uuidで一意な名前を作ること
- ファイル名がセットされていれば、それを返す。
たまにしか再現しないバグだったので、はまってしまいました。
Railsエンジニアのみなさん、気をつけてください。
読者登録をしていただけると、ブログを続ける励みになりますので、よろしくお願いします。