😇

Rubyのconcatが怖い(風評被害)

2022/12/21に公開

大遅刻です。

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 で返ります

で、 wherenil を渡すと、何も返ってこないクエリの完成です!(涙)

Active Recordでメソッド実行してたと思ってたけど、よくみたらインスタンス変数&メモ化されていた

よく理解しないまま進めるのはだめである(戒め)

    def fugafuga_ids
        @fugafuga_ids ||= OrderItem.joins(:hogehoge).where(hogehoge: { key: fugafuga_status: anywhere_status }).ids
    end

||= これはメモ化と呼ばれるテクニックとのこと

重たいクエリとか叩くとなると、毎回実行するたびに時間がかかるので、それをインスタンス変数に格納して、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を知ろう

はい

まだ途中ですが、これ読んでます。とてもいい本だと思っています。

https://gihyo.jp/book/2021/978-4-297-12437-3

頑張ってRubyの人になります。

Discussion