在rails中优雅的进行模型校验

      在用rails进行开发时，最常见的操作的是前台提交表单，后台保存相关的模型对象，对于一个复杂的表单，可能需要保存的模型对象不止一个，但这些对象在保存之前都是要经过合法性检验的，请看如下的场景，一个表单提交了订单信息、用户基本资料、用户所在地，那么后台需要对用户、订单、地点这3个对象进行校验并做相关处理，通常会用到error_messages_for做错误信息输出， 具体有以下3种做法:

 

1

if @user.save && @order.save && @city.save
  #跳转到成功页面
  else
  #返回原来页面
end

 

   这里存在的问题是，加入前两个模型合法性校验通过，但最后一个模型出现问题，那么原本3个对象都不该被保存，但前两个对象已经被保存，所以存在严重的脏数据问题

 

2

 

if @user.valid? && @order.valid? && @city.valid?
  @user.save
  @order.save
  @city.save
  #跳转到成功页面
else
  #返回原页面
end

 

      这个方法倒是不存在脏数据问题，但是如果第一个模型对象出现合法性问题，那么程序将停止之后的合法性校验，所以显示返回页面的错误提示将不完整，严重影响了系统的用户体验。

 

3 利用事务，基于第一种方法之上，如果任何一个模型合法性出现合法性问题，将采取数据库回滚操作，个人认为这种方法不仅复杂，性能也不高。

 

 

那么以下有个比较简洁的方法解决这样的问题

 

  def new
    @users = User.new
    @city = City.new
    @order = Order.new
  end
  
  def create
    @city = City.new params[:city]
    
    @user = User.new params[:user]
    @user.city = @city
    
    @order = Order.new params[:order]
    @order.user = @user

    unless [@user, @city, @order].map(&:valid?).include?(false)
      @user.save
      @city.save
      @order.save
      redirect_to "/main/new"
    else
      render :action => "new"
    end
  end

 

 

 关键在于这句：

 

 unless [@user, @city, @order].map(&:valid?).include?(false) 

 

在保存之前就遍历各个模型，并运行valid?方法，之后判断结果列表中是否包括false，以此作为判断合法性的依据，并且不会造成脏数据的问题。

 

 

相关的view如下所示，关于错误汉化这里不做讨论

 

<%= error_messages_for :user %> 
<%= error_messages_for :city %> 
<%= error_messages_for :order %> 

<% form_for :user, :url => "/main/create" do |f| %> 
  <fieldset>     
    <legend>用户信息</legend>     
    <ol>       
      <li>        
        <%= f.label :name %>        
        <%= f.text_field :name %>      
      </li>     
    </ol>  
  </fieldset>   
  <% fields_for :city do |city| %>     
    <fieldset>       
      <legend>地点信息</legend>      
      <ol>     
        <li>      
          <%= city.label :code %>   
          <%= city.text_field :code %>       
        </li>       
      </ol>     
    </fieldset>   
  <% end %> 

  <% fields_for :order do |order| %>  
    <fieldset>       
      <legend>订单信息</legend>     
      <ol>      
        <li>           
          <%= order.label :price %>          
          <%= order.text_field :price %>        
        </li>    
      </ol>     
    </fieldset>
  <% end %>
  <%= f.submit '提交' %> 
<% end %>

 

 

 

 

评论

55 楼 Hooopo 2009-09-07  

<div class="quote_title">liuqiang 写道</div>
<div class="quote_div">
<p> </p>
<div class="quote_title">gigix 写道</div>
<div class="quote_div">
<div class="quote_title">liuqiang 写道</div>
<div class="quote_div">呵呵，针对20个表单的过滤器，<span style="color: #ff0000;">代码肯定不止20行</span>，2者代码都不是很多，没必要斤斤计较啦，综合考虑，您再仔细琢磨琢磨，哪种方法比较爽</div>
<br>8行 <br><pre name="code" class="ruby">class TransactionFilter
  def filter(controller)
    return yield if controller.request.get?
    ActiveRecord::Base.transaction do
      yield
    end
  end
end</pre>
<br>你大概还需要2～3行把它声明在application.rb里面 <br>作为比较，我这个项目大概有150～200个action <br><br>重点在于： <br>（1）重复 <br>（2）不必要的思考</div>
<p> </p>
<p> 好吧，比吧，unless [@user, @city, @order].map(&amp;:valid?).include?(false)  这是个条件表达式，不算一行代码，所以我这个是0行代码 </p>
<p>注意，您还是把您这个方法的调用，也说明下</p>
<p> </p>
<p>PS:才发现我们的做法可能并不冲突</p>
</div>
<p>感觉问题不在性能方面，即使有文件操作也可以把上传图片操作放到下一步来做（实际上很多网站是这样做的）。</p>
<p>而真正的问题是如何让数据库事物保障和页面的错误回显这两个正交需求得到统一。</p>
<p>或许楼主的做法和gigix的结合一下会是一个完美的方案。</p>
<p><img src="/images/smiles/icon_wink.gif" alt=""></p>

54 楼 rainchen 2009-09-07  

Hooopo 写道

rainchen 写道

如果多个模型间是有关联的话，可以设置autosave：
Validation is performed on the parent as usual, but also on all autosave enabled associations. If any of the associations fail validation, its error messages will be applied on the parents errors object and validation of the parent will fail.


详细用法见：
http://api.rubyonrails.org/classes/ActiveRecord/AutosaveAssociation.html

如果没有直接关联的话，更优雅点的写法：

if [@user, @city, @order].all?(&:valid?)
  [@user, @city, @order].map(&:save)
end

这里用all?方法看似不错，可是却有一些问题，原因如下：
all?和any?方法很懒堕，不能把所有的model都验证一遍。
简单举个例子：

irb(main):007:0> [nil,1,nil].all?{|item| puts item.nil?}
true
=> false

返回结果是没有错，但是没有达到全部验证的目的，同样的any?也一样
所以这个需求用map和select是不错的选择。

all?是遵从短路法则，只要其中一个item是false，后面items的就短路了，既然其中一个都不能通过校验了，其他的也没必要校验了，当然，如果要一次过给出所有错误信息，那就collect 所有errors

53 楼 Hooopo 2009-09-07  

rainchen 写道

如果多个模型间是有关联的话，可以设置autosave：
Validation is performed on the parent as usual, but also on all autosave enabled associations. If any of the associations fail validation, its error messages will be applied on the parents errors object and validation of the parent will fail.


详细用法见：
http://api.rubyonrails.org/classes/ActiveRecord/AutosaveAssociation.html

如果没有直接关联的话，更优雅点的写法：

if [@user, @city, @order].all?(&:valid?)
  [@user, @city, @order].map(&:save)
end

这里用all?方法看似不错，可是却有一些问题，原因如下：
all?和any?方法很懒堕，不能把所有的model都验证一遍。
简单举个例子：

irb(main):007:0> [nil,1,nil].all?{|item| puts item.nil?}
true
=> false

返回结果是没有错，但是没有达到全部验证的目的，同样的any?也一样
所以这个需求用map和select是不错的选择。

52 楼 rainchen 2009-09-07  

如果多个模型间是有关联的话，可以设置autosave：
Validation is performed on the parent as usual, but also on all autosave enabled associations. If any of the associations fail validation, its error messages will be applied on the parents errors object and validation of the parent will fail.


详细用法见：
http://api.rubyonrails.org/classes/ActiveRecord/AutosaveAssociation.html

如果没有直接关联的话，更优雅点的写法：

if [@user, @city, @order].all?(&:valid?)
  [@user, @city, @order].map(&:save)
end

51 楼 Hooopo 2009-09-05  

liuqiang 写道

见你LS的LS的LS，已经说了

你感兴趣可以试下老王的方法，看看能不能把所有model的出错信息全部调出来。

快客王的做法只对他举的例子中的那种关联实用，对像下面的关联就没办法了：

@user = User.new(params[:user])
@account = @user.build_account(params[:account])
@profile = @user.build_profile(params[:profile])

至于是不是能把错误信息全调出来没有试验。。。

楼主的方法的确不错。但是unless ... else看着很不爽，稍微改了一下：

if [@user, @city, @order].select{|model| model.valid? == false}.empty?
   ...
else
   ...
end  

由于实际中可能需要加验证码，再提供一个带验证码（用的simple capatcha插件）的版本：

if mutiple_model_with_captcha_valid?(:user, @user, @city, @order)
   ...
else
   ...
end
 

#带验证码复合表单提交前验证
  def mutiple_model_with_captcha_valid?(with_captcha_model, *models)
    valid_results = models.map do |model|
      if ActiveSupport::ModelName.new(model.class.name).singular == with_captcha_model.to_s
        model.valid_with_captcha?
      else
        model.valid?
      end
    end
    valid_results.include?(false) ? false : true
  end

50 楼 lobbychmd 2008-11-07  

我觉得LZ 的方法挺好的，按分层的思想就是 业务层的ruby 做校验，减轻数据库服务器负担嘛。业务层可以做负载均衡，而DB 只有一个。比用事务做检查就先进些。

但有些特殊情况，可能必须在事务的中间或者结尾才能检查，例如可能把发货数量按先进先出分配到批次之后，才能校验，这时候还是利用事务本身作检验比较好些。

49 楼 blackwolf1983 2008-11-06  

gigix 写道

blackwolf1983 写道

gigix大牛，我想问一下，数据库回滚后如何定位错误字段

还有，你用java的那些框架，比如struts时，难道不用现成的验证工具吗？都是直接提交给数据库处理吗？

1. 回滚和定位错误字段是两个正交的需求
2. 验证工具和数据库事务是两个层面的正确性保证

您说话太专业了，我有些看不懂
1.正交的需求是不是说等数据库报错时再验证
2.两个保证，但我想不出来验证通过的数据为什么会在数据库端出错

48 楼 gigix 2008-11-06  

blackwolf1983 写道

gigix大牛，我想问一下，数据库回滚后如何定位错误字段

还有，你用java的那些框架，比如struts时，难道不用现成的验证工具吗？都是直接提交给数据库处理吗？

1. 回滚和定位错误字段是两个正交的需求
2. 验证工具和数据库事务是两个层面的正确性保证

47 楼 blackwolf1983 2008-11-06  

gigix大牛，我想问一下，数据库回滚后如何定位错误字段

还有，你用java的那些框架，比如struts时，难道不用现成的验证工具吗？都是直接提交给数据库处理吗？

46 楼 gigix 2008-11-06  

懒得引用了，因为这种事情从来就没有一概而论的最佳方案，你只是在精确的事务声明和节约开发成本之间权衡。如果你认为自己的场景有几百个通用情况和几个特殊情况，你可以考虑一种无伤大雅的通用方式让自己少考虑一个问题。如果你认为自己的场景全都是特殊情况，那么就去精确声明好了。

45 楼 robbin 2008-11-06  

gigix 写道

robbin 写道

对于程序员来说，这意味着编码标准的混乱。比方说你自己写一个model的方法，你现在就会搞不清楚究竟要不要加transaction block。加吧，很可能该方法被POST action调用，没必要重复加了；不加吧，万一被GET action调用了，没有事务保护了。得，最后还是统统加上block保险。但是这样一来，你的filter不是白写了吗？还不如去掉得了，无谓拉长了transaction scope。而实际上根据我的实践经验，长事务很容易造成数据库资源的锁定，应该是被极力避免的。

从REST的角度来看，model本来就不应该知道自己的一个操作是否应该需要事务保护，因为只有在资源的角度上你才能判断一个操作是否需要事务保护。至于性能问题，我很少在它没有出现的时候去考虑它。性能瓶颈出现的时候再特殊处理它好了，为了一个想象中的性能瓶颈害得几百个action不得不多考虑一件事情，我不认为这是个划算的生意。

第一、Rails框架的transaction block是定义在ActiveRecord::Base上，而不是ActionController上面，这已经是一个很明显的暗示了。

第二、我理解Rails的transaction block与其说是事务保护，不如说是“在代码里面进行显式的事务声明”，它声明了block内的代码需要事务范围。所以从这一点上来看，无论写model代码，lib代码还是action代码，只要程序员认为某段代码需要在一个事务范围内完成的时候，就应该加transaction block去显式的声明这一点。如果是我写代码，除非是单步事务，否则我会在model里面声明一次，然后在action里面调用model的时候再声明一次。而不应该因为有了一个统一的filter拦截web请求之后，放弃了在代码里面对事务的声明，这样的代码缺乏好的描述性。

第三、我们这里谈的事务毕竟是指数据库事务，而model需要不需要事务，是根据数据库操作来决定的，和REST无关，所以从REST的角度根本无从判断你需要不需要事务。就像我上面举的例子。从REST的角度来说访问本贴：GET /topics/238160，是不改变资源状态的，但是这里你不能得出不需要事务的结论。事实上因为我们需要记录用户访问轨迹，帖子点击次数，照样需要数据库事务。

第四、对于我们的应用场景来说我还是比较关注这个问题。因为数据库一旦出现IO瓶颈，是最棘手的问题，而commit操作会引起IO。

44 楼 gigix 2008-11-06  

robbin 写道

对于程序员来说，这意味着编码标准的混乱。比方说你自己写一个model的方法，你现在就会搞不清楚究竟要不要加transaction block。加吧，很可能该方法被POST action调用，没必要重复加了；不加吧，万一被GET action调用了，没有事务保护了。得，最后还是统统加上block保险。但是这样一来，你的filter不是白写了吗？还不如去掉得了，无谓拉长了transaction scope。而实际上根据我的实践经验，长事务很容易造成数据库资源的锁定，应该是被极力避免的。

从REST的角度来看，model本来就不应该知道自己的一个操作是否应该需要事务保护，因为只有在资源的角度上你才能判断一个操作是否需要事务保护。至于性能问题，我很少在它没有出现的时候去考虑它。性能瓶颈出现的时候再特殊处理它好了，为了一个想象中的性能瓶颈害得几百个action不得不多考虑一件事情，我不认为这是个划算的生意。

43 楼 robbin 2008-11-06  

gigix 写道

robbin 写道

Quake Wang 写道

robbin 写道

我非常反对gigix这种不管三七二十一，任何请求进来都统统最后给commit一把的做法。因为很早以前我就做过性能测试，对于每个请求都commit的做法，虽然对请求处理的速度影响很小(大约5%的性能损失)，但是对数据库会造成非常大的压力，会让数据库的CPU占用率直线上升。

gigix之前贴的代码里面是有判定非get request才进行transaction的open/commit，所以对数据库不会有影响的。

但有些GET也会有transaction的，比方说计算帖子点击啦什么的。GET请求另外写一套方案？

解决大部分情况，特殊情况特殊处理咯
那些地方难道不是本来也要写一个ActiveRecord::Base.transaction的吗？

对于程序员来说，这意味着编码标准的混乱。比方说你自己写一个model的方法，你现在就会搞不清楚究竟要不要加transaction block。加吧，很可能该方法被POST action调用，没必要重复加了；不加吧，万一被GET action调用了，没有事务保护了。得，最后还是统统加上block保险。但是这样一来，你的filter不是白写了吗？还不如去掉得了，无谓拉长了transaction scope。而实际上根据我的实践经验，长事务很容易造成数据库资源的锁定，应该是被极力避免的。

42 楼 gigix 2008-11-06  

robbin 写道

Quake Wang 写道

robbin 写道

我非常反对gigix这种不管三七二十一，任何请求进来都统统最后给commit一把的做法。因为很早以前我就做过性能测试，对于每个请求都commit的做法，虽然对请求处理的速度影响很小(大约5%的性能损失)，但是对数据库会造成非常大的压力，会让数据库的CPU占用率直线上升。

gigix之前贴的代码里面是有判定非get request才进行transaction的open/commit，所以对数据库不会有影响的。

但有些GET也会有transaction的，比方说计算帖子点击啦什么的。GET请求另外写一套方案？

解决大部分情况，特殊情况特殊处理咯
那些地方难道不是本来也要写一个ActiveRecord::Base.transaction的吗？

41 楼 robbin 2008-11-06  

Quake Wang 写道

robbin 写道

我非常反对gigix这种不管三七二十一，任何请求进来都统统最后给commit一把的做法。因为很早以前我就做过性能测试，对于每个请求都commit的做法，虽然对请求处理的速度影响很小(大约5%的性能损失)，但是对数据库会造成非常大的压力，会让数据库的CPU占用率直线上升。

gigix之前贴的代码里面是有判定非get request才进行transaction的open/commit，所以对数据库不会有影响的。

但有些GET也会有transaction的，比方说计算帖子点击啦什么的。GET请求另外写一套方案？

40 楼 QuakeWang 2008-11-06  

robbin 写道

我非常反对gigix这种不管三七二十一，任何请求进来都统统最后给commit一把的做法。因为很早以前我就做过性能测试，对于每个请求都commit的做法，虽然对请求处理的速度影响很小(大约5%的性能损失)，但是对数据库会造成非常大的压力，会让数据库的CPU占用率直线上升。

gigix之前贴的代码里面是有判定非get request才进行transaction的open/commit，所以对数据库不会有影响的。

39 楼 robbin 2008-11-06  

我非常反对gigix这种不管三七二十一，任何请求进来都统统最后给commit一把的做法。因为很早以前我就做过性能测试，对于每个请求都commit的做法，虽然对请求处理的速度影响很小(大约5%的性能损失)，但是对数据库会造成非常大的压力，会让数据库的CPU占用率直线上升。

38 楼 QuakeWang 2008-11-06  

gigix 写道

liuqiang 写道

注意，您还是把您这个方法的调用，说明下

class ApplicationController < ActionController::Base
  around_filter :transaction_filter
  def transaction_filter(&block)
    TransactionFilter.new.filter(self, &block)
  end

今天在google一个rails transaction问题的时候发现又回到了这篇，用gigix的这种方法会有一个问题：
rails的around_filter是在整个action执行完以后才会执行，这样事务边界就被扩大到了整个action

def foo
#希望有个改进的filter机制从这里开始
  model1.save
  model2.save
#在任何render之前结束
  render :action => :bar
#而不是象现在的around_filter在这里结束
end

回头再看看rails的代码，是否能够找个简单的地方hack一下。

37 楼 0701 2008-09-15  

<div class='quote_title'>gigix 写道</div>
<div class='quote_div'>
<div class='quote_title'>right now 写道</div>
<div class='quote_div'>另外我想问问gigix同学，你的方法中，在过滤器是不是要判断对哪个表单进行校验，怎么弄的啊？如果有些model的保存只涉及一个model也要去过滤下全修改全不修改吗？哎……头疼</div>
<br/>sigh <br/>以前Spring那么火的时候你们在干什么呢…真疑惑… <br/><span style='color: #ff0000;'>对于绝大多数action，你不需要知道里面出了”什么“错，你只需要知道只要里面出了错</span>，那么所有的数据库操作都应该回滚 <br/>对于少数action，你需要在回滚的同时再回调一些东西来收拾残局 <br/>就这么简单</div>
<p> </p>
<p> sigh ， 我当然要知道出什么错啦，我还指望返回错误信息给用户呢！</p>
<p> </p>
<p>LZ 和快克王给出的是错误校验的具体办法，你所说的是事务，你只不过是想说把这个校验放到filter中而已，这本身就是2回事。用了事务（当然是必须的），难道就不需要数据校验了不成？这个事情是省不掉的，但照你的意思，就是管他什么格式，只要知道出错了就可以了，我觉得这样太恐怖了。当然你可能又另有蹊径去解决，你还是说清楚吧（这种校验也是必要的），省得误导群众。</p>
<p> </p>
<p>PS：借用一句话，这年头谁也不比谁多懂一点，劝各位还是按gigix的做法实践一遍在发言吧</p>

36 楼 whb 2008-09-15  

<div class='quote_title'>gigix 写道</div>
<div class='quote_div'>
<div class='quote_title'>liuqiang 写道</div>
<div class='quote_div'>呵呵，针对20个表单的过滤器，<span style='color: red;'>代码肯定不止20行</span>，2者代码都不是很多，没必要斤斤计较啦，综合考虑，您再仔细琢磨琢磨，哪种方法比较爽</div>
<br/>8行<br/>
<pre name='code' class='ruby'>class TransactionFilter
  def filter(controller)
    return yield if controller.request.get?
    ActiveRecord::Base.transaction do
      yield
    end
  end
end</pre>
<br/>你大概还需要2～3行把它声明在application.rb里面<br/>作为比较，我这个项目大概有150～200个action<br/><br/>重点在于：<br/>（1）重复<br/>（2）不必要的思考</div>
<p> </p>
<p>比较欣赏这个方法。有点像简化版的声明型事务。</p>
<p> </p>