Rubyのconcatが怖い(風評被害)
大遅刻です。
CBcloud 2022アドベントカレンダーの11日目の記事です。
概要
- Railsで開発しているWebアプリで不具合があった
- 根本な原因はメソッドの扱い方が間違っていた
- Active Recordのメモ化されている変数を受け取って、
concat
を使ったことで、混乱が生じた
背景
Railsで開発されているWebアプリに機能を追加してほしいというタスクがありました。
内容は、「バッチでサービスの注文データを用意した条件で検索して、ヒットしたらSlackへ通知してほしい」という内容でした。
で、はじめてのRailsで不慣れながらもなんとかレビューをパスして、リリースしましたが、不具合が発生しました。
ローカルや検証環境では問題なかったのですが、本番環境で問題が発生しました。
書いたコード
こんな感じ
class OrderItem < ApplicationRecord
# メソッドいっぱいなので省略
def hogehoge_id
OrderItem.where(name: 'hogehoge').pluck(&:id)
end
def fugafuga_ids
@fugafuga_ids ||= OrderItem.joins(:hogehoge).where(hogehoge: { key: fugafuga_status: anywhere_status }).ids
end
# メソッドいっぱいなので省略
end
class BatchItem
def execute
target_ids = collect_condtions_ids
# 省略
end
def collect_condtions_ids
target_ids = OrderItem.fugafuga_ids
target_ids.concat([OrderItem.hogehoge_id])
# 省略
target_ids
end
end
- OrderItemから生えているいくつかのメソッドから必要なデータのidを集めてくる
- このとき最初に叩くメソッド
fugafuga_ids
でメモ化された変数が変えることをあまり理解していなかった(フラグ)
- このとき最初に叩くメソッド
- BatchItemが新規追加したクラスで、こいつを実行して集めてきた注文データに用意した条件を当てて、残ったらSlackへ通知する的なロジックを組みました
リリース後の不具合確認
- ローカルから本番データを参照するように設定して検証
- Active Recordのインスタンス変数の値がバッチ実行するたびに変化する
- その中に
nil
が含まれており、Active Recrodのwhere
に渡しているため、検索が死んでいた
この時点の僕の疑問
- なぜ
nil
が含まれているのか? - 検索結果がなぜ実行するたびに変化するのか?
原因
関連はこんな感じ
- なぜ
nil
が含まれているのか?- 一部関数がバグっていたことで、結果が
nil
になり、それにより検索が死んだ
- 一部関数がバグっていたことで、結果が
- 検索結果がなぜ実行するたびに変化するのか?
- Active Recordでメソッド実行してたと思ってたけど、よくみたらインスタンス変数&メモ化されていた
- 受け取った変数に、
concat
を実行して破壊的変更を与えていた
nil
になり、それにより検索が死んだ
一部関数がバグっていたことで、結果が nil
になる原因はただただ使いかたが間違ってただけでした
def hogehoge_id
OrderItem.where(name: 'hogehoge').pluck(&:id)
end
は、 pluck(&:id)
ではなく pluck(:id)
です。 Rails難しい(違
&
つけたまま実行すると nil
で返ります
で、 where
に nil
を渡すと、何も返ってこないクエリの完成です!(涙)
Active Recordでメソッド実行してたと思ってたけど、よくみたらインスタンス変数&メモ化されていた
よく理解しないまま進めるのはだめである(戒め)
def fugafuga_ids
@fugafuga_ids ||= OrderItem.joins(:hogehoge).where(hogehoge: { key: fugafuga_status: anywhere_status }).ids
end
||=
これはメモ化と呼ばれるテクニックとのこと
- https://qiita.com/kt215prg/items/3c0fd89468dcfe6075df
- https://kazumalab.hatenablog.com/entry/2018/12/03/235258
重たいクエリとか叩くとなると、毎回実行するたびに時間がかかるので、それをインスタンス変数に格納して、2回目は一度実行した結果を返すということ。要はキャッシュである。
今回ここの処理が実行するたびに変化したんですが、それは呼び出し側の concat
であることがわかりました。
concat
を実行して破壊的変更を与えていた
受け取った変数に、 def collect_condtions_ids
target_ids = OrderItem.fugafuga_ids
target_ids.concat([OrderItem.hogehoge_id]) # ここ
# 省略
target_ids
end
なるほど〜〜〜〜〜〜〜〜〜〜〜。インスタンス変数が変更されていたのか
おもむろにPythonさんを起動する
>>> class Hoge():
... def __init__(self):
... self.hoge = []
...
>>> h = Hoge()
>>> h.hoge
[]
>>> hoge = h.hoge
>>> hoge += [10, 20, 30]
>>> hoge
[10, 20, 30]
>>> h.hoge
[10, 20, 30]
まぁPythonさんもそうだったね。ということを思い出しました。言語に罪はなく、知らぬまま使う僕に責任が(ry
concat
の利用はレビュー時には指摘もらっていた(明確にバグという話ではなく、破壊的に変更する関数なので多様禁物的な)のですが、「今回は大丈夫!(全然大丈夫じゃない)」とコメント返して進めてしまったため、次回からは安全に利用できるように確認します。はい。。。
というわけで、実行するたびに結果が違うのは、インスタンス変数を受け取った変数に破壊的変更を行っていたため。ということがわかりました
修正
- バグっていた関数を修正
- 破壊的変更しない処理に修正
で、無事動き今運用されています(自分が開発した機能が周りに利用してもらっている&FBもらえるとても良い気持ちになりました)
雑感
- プログラム言語に罪はないので学びましょう
- Rubyを知ろう
- Railsを知ろう
はい
まだ途中ですが、これ読んでます。とてもいい本だと思っています。
頑張ってRubyの人になります。
Discussion