Railsでの実装で気をつけたい3つの罠

In: Ruby / Rails| technology

8 3月 2009

実はRailsでの開発を仕事とするようになって、そこそこ経ちました。
初めは、すげー、これ便利! と感動することしきりだったわけですけれど、いざ実運用に乗せてみると苦戦することもたくさん。
今回は、実装するときは、ちょっと立ち止まって考えないと、はまってしまいそうな(ていうか、よくはまってた)事例をまとめてみようかと思います。

NoMethodErrorの罠

まず一番よく出るのが、NoMethodError。
たとえば、モデルのリレーションに

belongs_to :user

と指定したとしましょう。
で、ビュー側では以下のように書いていたとします。

entry.user.name

もしテーブルのuser_idがnilのデータを取得してきたら、NoMethodErrorが出てしまいますね。
ちょっと気を抜くと、このエラーが頻発します。

if entry.user.blank? ? entry.user.name : "名無しさん"

たとえば、必ずこんな感じにでも書くようにする、とか決まっていればいいのかもしれませんが、実はnilの入る可能性のある項目なのに、ここは必ず入力されているもの、とか思い込むと意識から飛んだり、そもそもその可能性を見落としたりすることがよくあります。

 

フィルターの罠

2つめは、フィルター絡みの事例。
Railsでは、コントローラの処理の前後にフィルターを挟むことができます。
そうすることで、共通の処理の手間を省こうという狙いですね。
でも、使い方を間違えると、かえって痛い思いをしたりします。

class HogeController < ApplicationController
  before_filter :get_mobile_info
  before_filter :login_required
  before_filter :profile_required

  def index
    #(中略)
  end

  before_filter :create_access_log

  def search
    #(中略)
  end

  after_filter :update_users_logined_at
end

たとえば、これは極端な例かもしれないけれど、誰かとソースが競合するのが嫌だからって書く場所を変えたりすると、こんなコードになったりします。

こんな風に、フィルターの定義を一番上と一番下にでも書かれたら、それだけで追いかけるのが難しくなります。
まして、真ん中の辺りにでも書かれようものなら、8割がた見落とせます。
さらに、コントローラの継承までやっていて、そこにも同じようにフィルターが挟まっていたら、メソッドを実行する前に、何がどれだけ動くのか把握できたものじゃありません。間接的なスパゲッティコード(しかも読みにくい。。)の誕生です。

簡単に書けてしまうから、と、ついつい使ってしまうけど、バグが起きたときの追跡の手間とかを考えると、ひとつに集約できたらいいのになー、と思わずにはいられません。

before_filter :login_required, :available => [{:controller => :home, :except =>"login"},
                                              {:controller => :diary, :except => "show"}]

たとえばこんな感じで、どこかで一括指定でもできたらいいのに、と最近は思います。(何かしら、プラグインとかでありそうですが。。。)

 

トランザクションの罠

3つ目は、トランザクション。
Railsの場合、トランザクションは、特に何も指定しなければ、saveやらdeleteやらのタイミングで一個ずつ走ります。
そのため、場合によっては、いわゆる、プログラミング的トランザクションという奴を意識しないと、後々潜在的なバグに苦しめられることになります。

たとえば、自分の手持ちのポイントを払ってアイテムを買うような処理を考えてみます。
同じアイテムは一個しか買えないという仕様だったとしましょう。

class ItemController < ApplicationController
  def buy
    item = Item.find_by_id(params[:id])
    unless item
      raise NotFoundException
    end
    if UsersItem.has_been_bought?(item, @login_user)
      flash[:error] = "すでに購入済みですよ。"
      redirect_to :action => "check"
      return
    end
    users_item = UsersItem.new(:item_id => item.id, :user_id => @login_user.id)
    users_item.save
    buyer = User.find_by_id(@login_user.id)
    buyer.point = buyer.point - item.point
    buyer.save
  rescue NotFoundException => e
    flash[:error] = "そんなアイテムないですよ。"
    redirect_to :action => "check"
    return
  end
end
class UsersItem < ActiveRecord::Base
  def self.has_been_bought?(item, user)
    find_by_user_id_and_item_id(user.id, item.id)
  end
end

みたいな感じかと思われます(適当に書いたから間違ってる予感も。。)。

こう書いた場合、UsersItemsテーブルへのsave処理が何かの理由で待ち状態になってしまっているときに、もし再度同じメソッドが実行されたら、あっさり直前のチェックロジックを通り抜けて、アイテムがめでたく2つ買えてしまうことになります。

もちろん、前後をActiveRecord::Base.transaction do?endで囲って、さらにsave後にもチェックロジックを挟んで、いざ引っかかったときはraiseしたものをrescueすれば、おそらく改善されるのでしょう(長い)。ソースで書けばこんな感じでしょうか。

class ItemController < ApplicationController
  def buy
    item = Item.find_by_id(params[:id])
    unless item
      raise NotFoundException
    end
    if UsersItem.has_been_bought?(item, @login_user)
      flash[:error] = "すでに購入済みですよ。"
      redirect_to :action => "check"
      return
    end
    ActiveRecord::Base.transaction do
      users_item = UsersItem.new(:item_id => item.id, :user_id => @login_user.id)
      if users_item.save
        if UsersItem.count_by_item_id_and_user_id(item.id, @login_user.id) == 1
          buyer = User.find_by_id(@login_user.id)
          buyer.point = buyer.point - item.point
          buyer.save
        else
          raise DuplicatePurchaseException
        end
      else
        redirect_to :action => "check"
        return
      end
    end
  rescue NotFoundException => e
    flash[:error] = "そんなアイテムないですよ。"
    redirect_to :action => "check"
    return
  rescue DuplicatePurchaseException => e
    flash[:error] = "すでに購入済ですよ。"
    redirect_to :action => "check"
    return
  end
end

常にこれをやるのもあんまり賢くない感じがします。。
でも、厳密にやらなければいけないとき(たとえば、お金が絡むときとか)は、これくらい慎重でありたいものです。

きちんと動くようにするために

実はこういうのがまずい、こうしたほうがいい、というやり方は、運用を始めてみてようやくわかるのかもしれません。
そういうノウハウって、数をこなして、事例を経験していかないと蓄積されないんだろうなぁ。


1 Response to Railsでの実装で気をつけたい3つの罠

Avatar

BestZachery

8月 15th, 2018 at 03:42:59

I see you don’t monetize your page, don’t waste your traffic, you can earn additional bucks
every month. You can use the best adsense alternative for any
type of website (they approve all websites), for more details simply search in gooogle:
boorfe’s tips monetize your website

Comment Form

About this blog

ゆるーく、ふわーっと、興味のままに。

自分のかたわらに置いておくメモ代わり。

Photostream