脈絡はありません

Railsエンジニアです。ハードルは低いほうが飛びやすい。

RSpecでS3オブジェクトの挙動をスタブする

前回記事の続き。前回は1回のRakeタスクで発行されるクエリ数の削減に取り組んだ。

inoway46.hatenablog.com

やりたいこと

リファクタによってコードを壊さないために、RSpecでテストを追加したい。

Rakeタスク内でS3オブジェクトを使用しているため、これをスタブ化する。

スタブ化したい理由は2つ。

  • S3からのファイル取得による料金発生を防ぎたい
  • S3上のファイルではなくローカルのテストデータを使用したい

該当のコード

s3 = Aws::S3::Client.new(
  region: region,
  access_key_id: ENV['AWS_ACCESS_KEY_ID'],
  secret_access_key: ENV['AWS_SECRET_ACCESS_KEY']
)

ソースコード

file = s3.get_object(bucket: bucket, key: key)
lines = CSV.parse(file.body.read)

ソースコード

実装したコード

describe 'import_with_line' do
  subject(:task) { Rake.application['master_csv:import_with_line'] }

  it 'import_with_line' do
    file = file_fixture('master.csv').read
    allow_any_instance_of(Aws::S3::Client).to receive_message_chain(:get_object, :body, :read).and_return(file)
    expect { task.invoke }.not_to raise_error
  end
end

以下が今回の味噌となる部分。

allow_any_instance_of(Aws::S3::Client).to receive_message_chain(:get_object, :body, :read).and_return(file)

実装過程

前提

そもそもRakeタスクのコードが上から順番に処理していくようないわゆる手続き型になっているためテストするのが難しいが、 そこに手を加えるモチベがなかったので、ひとまずこのままでテストを書くことにした。

Aws::S3::Clientから生成されたs3インスタンスimport_with_lineタスク内で、get_objectメソッドを呼んでいる。

さらに、file.body.readというメソッドチェーンでデータの中身を読み込んでいる。

file = s3.get_object(bucket: bucket, key: key)
lines = CSV.parse(file.body.read)

(ちなみにCSV.parseではCSV形式の文字列を配列に変換している)

CSV.parse (Ruby 3.1 リファレンスマニュアル)

テスト環境の設定にもよるが、このままテストを実行するとS3上の実ファイルが読み込まれるか、もしくはS3への接続エラーが発生する。

どちらのケースにしても避けたいので、s3インスタンスをスタブ化してこちらで戻り値を指定するようにする。

手順

改めて実装したテストコードを貼る。

describe 'import_with_line' do
  subject(:task) { Rake.application['master_csv:import_with_line'] }

  it 'import_with_line' do
    file = file_fixture('master.csv').read
    allow_any_instance_of(Aws::S3::Client).to receive_message_chain(:get_object, :body, :read).and_return(file)
    expect { task.invoke }.not_to raise_error
  end
end

file = file_fixture('master.csv').readでは、spec/fixtures/files配下に置いたローカルデータを読み込んでいる。(このpathはRSpecのデフォルト設定だったはず)

続いて今回のメインテーマ。

allow_any_instance_of(Aws::S3::Client).to receive_message_chain(:get_object, :body, :read).and_return(file)

それぞれの引数で指定している内容は以下である。

allow_any_instance_of(クラス名).to receive_message_chain(:インスタンスメソッド群).and_return(戻り値)

これで、指定したクラスから作られた全てのインスタンスに対して、指定したメソッドチェーンを実行すると、設定した戻り値を返すことができるようになる。

ちなみに単一のメソッドを指定する場合は以下でOK。

allow_any_instance_of(クラス名).to receive(:インスタンスメソッド).and_return(戻り値)


マッチャはひとまずexpect { task.invoke }.not_to raise_errorという形でエラーが発生しないことを指定しているが、 調整後、以下のように変更する予定だ。

expect { task.invoke }.to change { LineNotification.count }.from(0).to(1)


[追記]

書き忘れていたが、Rakeタスクのコードをメソッドチェーン形式になるように微修正した。

file = s3.get_object(bucket: bucket, key: key).body.read
lines = CSV.parse(file)

終わりに

今回はallow_any_instance_ofを使用して、全てのインスタンスに同じ挙動を指定しているが、あまり柔軟性がないように思う。

ただ、色々と調べて試したが、Rakeタスク内のs3インスタンスの挙動を上書きする方法がこれ以外に思いつかなかった。

もしかすると以下記事の方法を上手く使えばよりスマートに書けるのかもしれない。少なくともmodel_specではこちらの方法がベターだろう。

例えばrspecでテストする場合は、allow(Aws::S3::Client).to receive(:new).and_return(client)のように常にスタブしたclientを返すでも良いかもしれません

qiita.com

client = Aws::S3::Client.new(stub_responses: true)
client.stub_responses(
  :list_objects_v2, {
    contents: [
      {key: 'test/my-directory-object', size: 0},
      {key: 'test/my-object', size: 100},
    ]
  },
)

bucket = Aws::S3::Bucket.new(name: 'test', client: client)
# スタブしたclientを使う
# 例えばrspecでテストする場合は、allow(Aws::S3::Client).to receive(:new).and_return(client)のように常にスタブしたclientを返すでも良いかもしれません

objects = Aws::S3::Bucket.new(name: 'test', client: client).objects(prefix: 'dummy')
objects.map{|object| [object.key, object.size]}
#=> [["test/my-directory-object", 0], ["test/my-object", 100]]

実装前はモックとは?スタブとは?というところからよく分かっていなかったので、とても学びになった。(記事内のスタブという言葉の使い方が間違っている恐れすらある)

こうすればいいんじゃないか的なコメントはいつでも大歓迎です。

読んでいただきありがとうございました。

〜第2回おわり〜

【第1回】いにしえの個人開発アプリをリファクタする

今回のお題

技術ブログのネタをどうしようかと迷った挙句、1年前にRailsで開発したアプリに手をつけることにした。

改良ポイントは果てしなくあるので、シリーズものにできる。我ながらナイスアイデア

ソースコード

github.com

制作期間は2021年11月~2022年2月で、未経験からWebエンジニアに転職するためのポートフォリオにしていた。

1年弱の実務経験を経た今の状態でコードを眺めてみると、色々な気づきがあって面白い。

長くなりそうなので前置きはこの辺にして、早速コードについて書いていきたい。

本題

▼今回作成したプルリク

github.com

対象はアニメの最新話が追加されたらLINE通知を送るバッチ処理

before

手をつけたいのは、深すぎるネストの最深部にある if LineNotification.can_notify?の箇所だ。

masters.each do |master|
  @contents = Content.where(master_id: master.id, line_flag: true)
  if @contents.present?
    @contents.each do |content|
      if content.episode < master.episode
        line_users = content.users.where.not(uid: nil)
        line_users.each do |user|
          message = {
            type: 'text',
            text: "#{master.title}の#{master.episode}話が公開されました!\n#{master.url}"
          }
          client.push_message(user.uid, message) if LineNotification.can_notify?
          LineNotification.create_record(master, month)
          p "LINE通知:#{content.title}をuser_id:#{user.id}さんに送信しました"
        end
      end
    end
  end
end

ソース:https://github.com/inoway46/subani/blob/596f8abb17a3eb4b99cc06b6f58348beca906997/lib/tasks/master_csv.rake#L114

このクラスメソッドはModel内で以下のように定義されている。

class LineNotification < ApplicationRecord
  belongs_to :master
  counter_culture :master

  scope :monthly_total, -> { where(month: Date.today.month) }

  def self.can_notify?
    monthly_total.size < 1000
  end

  def self.create_record(master, month)
    create(master_id: master.id, month: month)
  end
end

ソース:https://github.com/inoway46/subani/blob/596f8abb17a3eb4b99cc06b6f58348beca906997/app/models/line_notification.rb#L7

monthly_totalというscopeで、今月の通知数を取得し、1000件を超えていないかを判定している。 課金の発生を避けるためだ。(タダ乗りしてごめんなさい)

参照:料金プラン|LINE for Business

クエリの発行数を減らしたい

ここで気づくべきは、each処理の中で同じクエリを何度も発行してしまっていることだ。

line_users.each do |user|
  message = {
    type: 'text',
    text: "#{master.title}の#{master.episode}話が公開されました!\n#{master.url}"
  }
  client.push_message(user.uid, message) if LineNotification.can_notify?
  LineNotification.create_record(master, month)
  p "LINE通知:#{content.title}をuser_id:#{user.id}さんに送信しました"
end

通知対象のユーザー数が増えれば何十回、何百回と同じクエリが発行されてしまう。

そこで、monthly_totalの実行は1回にして、通知処理を実行するごとに、カウントを1ずつ増やすようにしてみることにした。

(他にもベターなやり方はおそらくあって、そもそもこの深すぎるネストをなんとかするべきでは?という大勢の声が聞こえる)

after

結果として出来上がったのが以下のコードだ。

class LineNotification < ApplicationRecord
  attribute :total_count, type: Integer, default: 1000

  belongs_to :master
  counter_culture :master

  scope :monthly_total, -> { where(month: Time.zone.today.month) }

  def self.create_record(master, month)
    create(master_id: master.id, month: month)
  end

  def can_notify?
    total_count < 1000
  end
end
#月に1000件以上プッシュ通知を送ると課金されるため通知数を管理
line_notification = LineNotification.new
line_notification.total_count = LineNotification.monthly_total.size

catch(:exit) do
  masters.each do |master|
    @contents = Content.where(master_id: master.id, line_flag: true)
    if @contents.present?
      @contents.each do |content|
        if content.episode < master.episode
          line_users = content.users.where.not(uid: nil)
          line_users.each do |user|
            if line_notification.can_notify?
              message = {
                type: 'text',
                text: "#{master.title}の#{master.episode}話が公開されました!\n#{master.url}"
              }
              client.push_message(user.uid, message)
              LineNotification.create_record(master, month)
              p "LINE通知:#{content.title}をuser_id:#{user.id}さんに送信しました"
              line_notification.total_count += 1
            else
              throw :exit
            end
          end
        end
      end
    end
  end
end

差分:https://github.com/inoway46/subani/pull/64/files

さらにネストが深くなってしまった。多重ループを抜ける方法は、throw→catchが良いと聞きまして・・・。

本題に戻ると、LineNotificationモデルにtotal_countという属性値を追加したことに注目いただきたい。 (デフォルト値を1000にしたのは何が何でもお金を払いたくないからです)

class LineNotification < ApplicationRecord
  attribute :total_count, type: Integer, default: 1000

バッチ処理内では、ネストの森の入り口で以下のように通知数を取得するようにした。

line_notification = LineNotification.new
line_notification.total_count = LineNotification.monthly_total.size

ギリギリ1000件までは通知するようにしたいので、多少の無駄クエリは発生するが、 最深部のループでif line_notification.can_notify?を判定するようにしている。

line_users.each do |user|
  if line_notification.can_notify?
    message = {
      type: 'text',
      text: "#{master.title}の#{master.episode}話が公開されました!\n#{master.url}"
    }
    client.push_message(user.uid, message)
    LineNotification.create_record(master, month)
    p "LINE通知:#{content.title}をuser_id:#{user.id}さんに送信しました"
    line_notification.total_count += 1
  else
    throw :exit
  end
end

通知処理に入ったら、送信後にline_notification.total_count += 1でカウントを追加する。

1000件を超えている場合は、throw :exitでループを一気に抜ける。

まとめ

以上が今回の変更内容である。

コード量は増えてしまったが、問題のクエリの発行数は1回に済ませることができた。

ちなみに、単にtotal_count = LineNotification.monthly_total.sizeとせずにtotal_countをモデルの属性値として追加した理由は、 コードの可読性を上げるためだ。

if total_count < 1000よりif line_notification.can_notify?の方が処理の意図伝わりやすくない?

そんなことないっすかね・・・。(自信なくなってきた)

ともあれ、少しでもパフォーマンスを上げられたので、今回はこれで良しとする。

PRをマージする前にテストコードを追加しようと思っているので、次回はその件について記事にしようと思う。

(テストを書くのが面倒になったらテーマを変更する)

最後までお付き合いいただきありがとうございました。

〜第1回おわり〜

技術ブログを書くと何がいいのか

Webエンジニアとして実務でコードを書き始めてから10ヶ月が経とうとしている。

プログラミングを本格的に学び始めたのは2020年夏からだから、学習期間は約2年になる。

 

これまではインプット中心の学習で、あまり積極的に外部への情報発信をやってこなかった。やったとしてもNotionにメモとしてまとめたり、Scrapboxでちょっとしたものを公開したりぐらいだ。

scrapbox.io

 

はてなブログでの記事公開は上記の媒体よりもオープンな発信になるだろう。このようなオープンな場で技術について書くことは、自身の成長にとってどのような効果があるだろうか。

 

まずは文章化する過程で、知識の不足に気づけること。オブジェクトってなんだろう、バイナリってなんだろう、インフラが指す範囲はどこからどこまで?

 

その他には、記事を公開した後に「あれ、あの内容って正しいんだっけ」という思考が脳内で回り出すことが考えられる。

間違ったことを書いてバカだと思われたくない、どこかのギークからマサカリを投げられたくないという感情から、さらなる情報のリサーチをするようになりそうだ。

 

いま所属している会社ではアウトプットが推奨されていて、評価にも直結している。

毎日、業務終わりに日報を書く中で、たしかに他者が見えるところで情報発信することは1人で黙々とインプットしているよりも成長に繋がる実感がある。(上述の理由から)

 

ただ、今はまだ息を吐くようにアウトプットする感じにはなれていないので、まずは簡単なテーマから始めて、アウトプットすることを癖づけていきたい。

 

所信表明的なこともできたので、この辺で終わっておきます。