Rails 出了问题靠猜?请看菜鸟是如何填坑的

victor · 2013年10月17日 · 最后由 liwei78 回复于 2013年10月18日 · 3790 次阅读

今天小伙伴需要的功能是:API 返回的数据,要有一个总数字段。这样他在手机端就可以判断,如果已经查看到最后一组产品了,往下拉动就不再去服务器请求数据了。为服务器减少不必要的请求压力。

嗯,看起来很完美,而且实现起来似乎很简单的样子。我回了他一句,妥妥的,给我 30 分钟。然后起身温盏,洗茶,泡了一盏茶之后,盘腿坐好,点开 focus booster 。打算在一个番茄时间内搞定一切。

我完全没过大脑,在 controller 里面写下了如下的代码

resources :products do
  desc "Listing of products."
  params do
    optional :tag, type: String, desc: "Tag name."
    optional :page, type: Integer, desc: "Page number."
    optional :per, type: Integer, default: 10, desc: "Per page value."
  end
  get "/", jbuilder: 'products/products' do
    @products = Product.tagged_with(tags, :any => true).page(params[:page]).per(params[:per])
    @total_number = Product.tagged_with(tags, :any => true).count
  end
end

然后在模板中添加

json.products @products do |product|
  json.partial! "products/product", product: product
end
json.total_number @total_number

恩,看起来我已经完成了。随手点两下,不报错。提交算了,不用写测试了。茶叶刚好能喝,我喊小伙伴去测试 API。我就喝茶吧。

慢吞吞喝完茶,小伙伴给了反馈。total_number 返回的数据比实际 products 数组的多。total_number 显示 8,可 products 才返回 7 条。擦,我又仔细看了看我的代码。直觉没啥错误啊。

加了一个断点,打算把程序打断看看情况。我用的是 pry

get "/", jbuilder: 'products/products' do
  binding.pry
  @products = Product.tagged_with(tags, :any => true).page(params[:page]).per(params[:per])
  @total_number = Product.tagged_with(tags, :any => true).count
end

再次刷新 API 的请求 URL 之后。程序卡在断点了。我先去掉了分页,自己执行了一下

products = Product.tagged_with(tags, :any => true)
total_number = Product.tagged_with(tags, :any => true).count
products.size

结果果然不同,total_number 返回 8,products.size 返回 7。

我的程序是跑在 Rails4 和 Ruby2 以及 Mysql5.5 上的。最先怀疑的是 Product 用了某些 Gem 是为 Rails3 写的,没有兼容 Rails4。所以我过滤了一遍 Gemfile

gem 'acts-as-taggable-on'
gem 'paranoia', '~> 2.0'

product 模型只用了这两个 gem,我回断点的地方粗略的扫了一眼生成的 sql 都正确的加入 WHERE `products`.`deleted_at` IS NULL 这个条件了。莫非 size 和 count 在新版本的 rails 和 ruby 中做了什么了不起的修改。是我们不知道的?所以我试着

products.count
products.size

一个 8 一个 7。看看 语句 A Product.tagged_with(tags, :any => true)语句 B Product.tagged_with(tags, :any => true).count 生成的 sql 有啥区别吧。

语句 A 结果如下

SELECT DISTINCT products.* FROM `products` JOIN taggings produ_taggings_652eb4a ON produ_taggings_652eb4a.taggable_id = products.id AND produ_taggings_652eb4a.taggable_type = 'Product' WHERE `products`.`deleted_at` IS NULL AND () ORDER BY rank DESC

语句 B 结果如下

SELECT COUNT(*) FROM `products` JOIN taggings produ_taggings_243b36e ON produ_taggings_243b36e.taggable_id = products.id AND produ_taggings_243b36e.taggable_type = 'Product' WHERE `products`.`deleted_at` IS NULL AND ()

差了一个 DISTINCT。看起来如果使用 count 方法的话,果然有我这个菜鸟不懂的地方啊。打开 Dash 看看文档吧。定位到 rails4 的文档然后输入 count 之后。出现将近 30 个备选项。立刻想跪了。耐心从第一个往下捋顺吧。看到 Calculation 这个词。好像跟计算有关,点开看看。

Person.count
# => the total count of all people

Person.count(:age)
# => returns the total count of all people whose age is present in database

Person.count(:all)
# => performs a COUNT(*) (:all is an alias for '*')

Person.distinct.count(:age)
# => counts the number of different age values

看来我需要加一个 distinct。废话不多说,加上试试看。听说如果少取几个字段还能优化 sql 的性能,真的吗?我也试试吧。

@products = Product.tagged_with(tags, :any => true).page(params[:page]).per(params[:per])
@total_number = Product.tagged_with(tags, :any => true).select(:id).distinct.count

结果正确了,俩都是 7。

完,番茄钟结束。起来溜达一圈。

我就想说,要是写了测试,至少这种问题都不会出现。

板凳,养着回来看

问题在 count 上??

从这个情景看,不应该是 api 端的问题。。。你已经返回全部数据了,这个如果给个标注,终端也不用去请求了。。。终端请求了,说明它认为还有记录。。。那么,它请求的时候,应该问:我已经拿到 38 个了,还有么?你说:38 个已经是最大了,没有了。或者你说:38 个不是最大,后面还有几个。

以此类推。

#1 楼 @nightire 写测试仍然会出现这个问题。这个问题产生的原因主要是因为我太菜,写了测试也只是提前帮助自己发现问题。不写测试就是小伙伴帮我发现问题。写不写测试都无法改变我是菜鸟的事实。

这么想,分页到最后一页,page=38,如果再加上个 下一页,或者你干脆请求 page=438,情况会如何??

#5 楼 @Victor 那倒也是,不过这也看测试怎么写了。测试虽然不能保证百分之百无 Bug,但至少可以提前帮你测试出返回值的正确与否,特别是如果事先用测试覆盖一下用例的结果集,那么像这类问题至少可以在交给同事验收之前就发现了。

写测试不是说保证你不出问题,而是能帮助你发现本来发现不了的问题,这总比每次都是别人给你指出来要更有效率的多,解决问题的思考和收获也都是自己吃透的了。

SELECT DISTINCT products.* FROM 这句话有什么不妥么??

#8 楼 @liwei78 这句是正确的,我以为 count 默认就会加上这个参数,而实际上没有。

#10 楼 @Victor @total_number = Product.tagged_with(tags, :any => true).select(:id).distinct.count 求它打印出来的东西。

菜鸟:东西打完就丢出去了,然后各种 bug 被打回来 熟手:东西打完上各种测试,然后没有预料到的 bug 被打回来 牛人:东西做之前考虑到各种状况以及处理,加上测试文档,和相关人士通气该功能使用注意事项以及各种需要注意的问题,丢出去什么事情都没有发生正常执行,然后月底写出一份 slide 详细汇报和介绍自己这个模块以及自己多么牛逼。

#11 楼 @liwei78 你问的那句打印出

SELECT DISTINCT COUNT(DISTINCT `products`.`id`) FROM `products` JOIN taggings produ_taggings_643d77b ON produ_taggings_643d77b.taggable_id = products.id AND produ_taggings_643d77b.taggable_type = 'Product' WHERE `products`.`deleted_at` IS NULL AND ()

另外

Product.tagged_with(tags, :any => true).pluck('DISTINCT products.id').count

打印出

SELECT DISTINCT products.id FROM `products` JOIN taggings produ_taggings_9f79315 ON produ_taggings_9f79315.taggable_id = products.id AND produ_taggings_9f79315.taggable_type = 'Product' WHERE `products`.`deleted_at` IS NULL AND ()

都是返回正确的 7

第二句是在一个 array 上做了 count,效率远低于第一句。

Pluck returns an Array of attribute values type-casted to match the plucked column names, if they can be deduced. Plucking an SQL fragment returns String values by default.

#14 楼 @liwei78 哥,我觉得第 2 句它也是一条 sql。还没有生成 array 呢。效率和第一条差不多吧。

Model.count 方法才是 select count(*) 方法。

谁能解释下 Model.method.method.method 的方法是怎么实现的???我总觉得这种 靠 . 来拼接查询的方法不可靠呀。。。

#15 楼 @Victor SELECT DISTINCT COUNT(DISTINCT products.id) FROM products; 和 SELECT DISTINCT products.id FROM products ;

执行下。。。

胸闷,气短,怎么破???我睡觉了。。。哎。。。。

#17 楼 @liwei78 我要是能破,我就不是菜鸟了。我也睡了。

#18 楼 @Victor 睡什么,继续讨论啊,我喜欢看。。

@Victor 大哥你这毁我五官啊,你这是用了 Kaminari 了吧。 你分页后 @products = Product.tagged_with(tags, :any => true).page(params[:page]).per(params[:per]) 可以直接调他的总数的 @products.total_count

#20 楼 @hbin 人艰不拆,求别说

#5 楼 @Victor #1 楼 @nightire

写测试 跟出不出问题 没关系吧

#22 楼 @themorecolor 是的,测试只是提前发现问题。即便他写了 100% 覆盖度的测试,可能也会犯这个错误,因为它写测试的时候就写错了,几遍跑通了测试也是错的。 人格分裂者可以把测试写的很好,他那么正常,不行滴。

大神 你能来个总结么 从小语文不好 概括能力就是个渣儿

#24 楼 @shooter AR 计算总数用 count 的话,想想再用。根据情况可能需要 distinct

我的情况是,应该按照 #20 楼 的说法直接用 total_count

叫你不写测试;写测试话,连点击页面都不用

#25 楼 @Victor 其实按照您的需求,您应该返回的是 @products.current_page@products.total_pages. 这样客户端用的时候就不需要再去计算一次有没有下一页。直接比较这两个数就出来了。

#27 楼 @hbin 客户端分页 走的是 Kaminari 么 貌似跟页面分页不一致吧

current_page 和 total_pages 需要自己计算出来吧

那啥,习惯上 API 设计的时候分页信息这种通用的东西不是应该放在 HTML 头里面的么,然后在 application_controller 里弄个after_action: :set_pagination_headers, only: :index 什么的

#26 楼 @mouse_lin 求别说。T_T #27 楼 @hbin 谢谢,新技能 Get √ #29 楼 @aptx4869 能详细讲讲吗?我是菜鸟。我第一版本的 API 是直接参考 http://railscasts.com/episodes/350-rest-api-versioning 写的。当时没想到会搞这么大一摊子。觉得能用就行。有些参数比如 device_id 就放在 request header 里面了。但是几个月后,移动 APP 的 API 请求达到百万级别的时候。想用 caches_action 做缓存。发现参数如果不是在 url 中,是没办法缓存的。

#29 楼 @aptx4869 如果你比较忙。请问你说的这种设计 API 的习惯,是从什么文章上看到的。能分享一下吗?谢谢

#31 楼 @Victor 搭积木到一定时候,就要拆掉重搭。。。。。。小孩纸会大哭一场。。

小心 distinct 过滤掉了某些数据。

#32 楼 @liwei78 我昨天遇到的那个问题,#25 楼 给的方案是我觉得最好的。

#31 楼 @Victor API 设计的时候显然要参考 GitHub 的 API,人家做的是相当标准…… 分页信息放在 Response Header 里应该算是最佳实践了吧,这样不需要给每个 index 模板都重复塞个 json.page 之类什么的,至于缓存,返回的 Response Header 应该没影响啊,而 Request header 本来就不应该放对缓存有影响的参数吧……

然后 set_pagination_headers 这方法是这里挖到的:

http://metabates.com/2012/02/22/adding-pagination-to-an-api/

上面这个是用 will_paginate 的,kaminari 应该需要稍微改一下

#34 楼 @aptx4869 长知识了。。。

需要 登录 后方可回复, 如果你还没有账号请 注册新账号