In: Ruby / Rails| technology
8 3月 2009実はRailsでの開発を仕事とするようになって、そこそこ経ちました。
初めは、すげー、これ便利! と感動することしきりだったわけですけれど、いざ実運用に乗せてみると苦戦することもたくさん。
今回は、実装するときは、ちょっと立ち止まって考えないと、はまってしまいそうな(ていうか、よくはまってた)事例をまとめてみようかと思います。
まず一番よく出るのが、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
常にこれをやるのもあんまり賢くない感じがします。。
でも、厳密にやらなければいけないとき(たとえば、お金が絡むときとか)は、これくらい慎重でありたいものです。
実はこういうのがまずい、こうしたほうがいい、というやり方は、運用を始めてみてようやくわかるのかもしれません。
そういうノウハウって、数をこなして、事例を経験していかないと蓄積されないんだろうなぁ。
ゆるーく、ふわーっと、興味のままに。
自分のかたわらに置いておくメモ代わり。