明けましておめでとうございます

新年ですね。下條です。年末タイに来て、現在はバンコクで働いておりまして、今週末日本に帰る予定です。バンコクに着いたのは明け方前で、タクシーで久しぶりにボラレてしまいました。集中力が欠けてる深夜こそ気を抜いてはいけないなあと思った次第です。

昨年を振り返って

技術的な面では、主にRuby on RailsでのWebアプリケーション、API開発、そして少しAndroidアプリ開発に携わった。もともとはC、C++、C#などでミドルウェアを開発してきた生粋のエンプラ人間だった私だったが、だいぶWeb人間に変わってきた気がする。いい意味で変わったのならいいのだが。

また、会社のメンバーも増えてきた。弊社は基本的に完全リモートワークであるので、メンバーが増えてきた場合にどうなるのかいろいろ楽しみである。おそらく、いろいろとやり方を変えていかないといけないと思うが、そういった変化を楽しみながら柔軟にやっていきたいと考えている。

そしてこれは個人的な話だが、先月に趣味で作った初のiOSアプリをAppleに申請した。アプリへのデータ配信用APIサーバはRailsでアプリを作ってDigitalOceanを使って立てた。DigitalOceanはRailsアプリのファイルをコピーするだけですぐにサービスを動かせるし安いので、時と場合によっては非常に便利である。大変に見た目がしょぼい(機能は素晴らしいと思っている)アプリなのだが、とりあえず審査に出してみて反応を見ようと思って出したところ、瞬殺リジェクトされてしまった。冬休みに修正版を出したので、私個人の初iOSアプリがリリースされるのが楽しみな今日この頃である。

今年の意気込み

今年はRuby on Railsのスキルを伸ばしつつ、フルスタックエンジニアを目指すべく?????積極的にスマートフォンやWebクライアントサイドの開発にも関わっていきたいと思っている。他にも、前職でやってきたJavaや.NETは一応得意分野であるので、そこらへんの受託案件をいただけばぜひ全力で取り組みたいと考えている。

本題(といいつつもちょっと小ネタ)

さて本題であるが、新年っぽくない地味なネタとなってしまうが、RailsのActiveRecordで最近ちょっとハマった小粒ネタを2つ書こうと思う。

Rails、MySQLのバージョンは以下。

  • Ruby on Rails 4.1.8
  • MySQL 5.6.17

find_byメソッドの引数が。。。

最近作りこんだバグのお話。

ActiveRecordのfind_byメソッドに主キーを渡してモデルを検索しようとした際、本来ハッシュを渡すべきところで、誤って主キーの値のみを渡していたコードがあった。

1
2
3
4
5
6
SomeModel.create(id: 1, some_column: 'hoge')
SomeModel.create(id: 2, some_column: 'fuga')
SomeModel.find_by(id: 2) →正しいコード
=> #<SomeModel id: 2, some_column: "fuga">
SomeModel.find_by(2) →今回書いてしまっていたバグってるコード
=> #<SomeModel id: 1, some_column: "hoge">

非常に単純なバグなのだが、、、
テストコードで検出できず、発見が遅れた。
上記の例では、find_by(id: 2)がid:2のレコードを返しているのに対し、find_by(2)はエラーとはならず、id:1のレコードを返している。

SomeModel.find_by(2)で発行されるSQLを見てみると、

1
SomeModel Load (0.3ms)  SELECT  "some_models".* FROM "some_models"  WHERE (2) LIMIT 1

これは当然ながら意図したクエリではない。しかし、エラーとならずに結果が返ってくる。そして、主キーが’2’のレコードではなく、全件引っかかった中のLIMIT 1で一番最初のレコードが返ってくる。

エラーにならずに結果がちゃんと1件返ってくるというのがちょっといやらしくて、テストコードは通ってしまっていた。というのも、テストコードではオブジェクト(レコード)をひとつしか作っておらず、find_byメソッドに誤った引数を与えていても意図せずに正しいレコードが引けてしまっていたためである。
まあ、テストコード以前に、ちゃんと動かして気付かない私が悪いという話であるのだが。。。

なお、find_byメソッドでなくfindメソッドは、主キーのみを渡して検索する。ハッシュは受け付けないので、これは間違えることはないだろう。

1
2
3
4
SomeModel.find(2)
=> #<SomeModel id: 2, some_column: "fuga">
SomeModel.find(id: 2)
=> TypeError: can't cast Hash to integer......

Rails 4.2ではfind_byの実装はだいぶ変わっているようだが、この件に関しては同様のようである。

というわけで、そんなん間違えねえよ!っていう声が聞こえてきそうですね。なんかバグを作りこんだ言い訳をしてるようで恐縮なのですが、find_byに渡す引数にはちょっと気をつけようというお話でした。

ネストしたモデルの属性更新時にユニーク制約でハマった問題

ActiveRecord関連でもう一つちょっと困ったネタ。

例えばこんなモデルがあった場合。

1
2
3
4
5
6
7
8
9
class Parent < ActiveRecord::Base
has_many :children
accepts_nested_attributes_for :children
end

class Child < ActiveRecord::Base
belongs_to :parent
validates_uniqueness_of :name
end

Parentモデルからupdate_attributesで複数のChildモデルのnameを一度に更新することができるが、「validates_uniqueness_of :name」を書いてあっても、DBにユニーク制約を入れていない場合、以下のように一度に同じ名前を登録することができてしまう。

1
2
3
4
5
6
7
8
9
10
11
12
?> p = Parent.create
=> #<Parent id: 3, created_at: "2011-06-08 17:31:54", updated_at: "2011-06-08 17:31:54">
>> p.update_attributes :children_attributes => [{:name => "Kid"}, {:name => "Kid"}]
=> true
>> Child.all.map(&:name)
=> ["Kid", "Kid"]
>> c = Child.first
=> #<Child id: 4, parent_id: 3, name: "Kid", created_at: "2011-06-08 17:32:12", updated_at: "2011-06-08 17:32:12">
>> c.save
=> false
>> c.errors.full_messages
=> ["Name has already been taken"]

つまり、’validates_uniqueness_of’をつけていてもchildren_attributesで一度に同じ値に更新された場合にはバリデーションは効かない。

これはaccepts_nested_attributes_forに限った話ではなく、validates_uniqueness_of一般に言える話である。

1
2
3
4
5
p = Parent.new
p.children << Child.new(:name => "Kid")
p.children << Child.new(:name => "Kid")
p.save
=> true

調べてみると、同じように困っている人がRailsのissueでたくさん見つかった。
https://github.com/rails/rails/issues/1572 (上記2つのサンプルコードは、このissueから流用した。)
https://github.com/rails/rails/issues/4568

そして問題を解決するというPRもあるのだが、最終的にマージはされていない。結局、複数ユーザからの更新などを考えると、DB側にユニーク制約を入れる必要がある。どこまでRails側で救うかという話で、今回の場合、複雑なコードを入れ込む割には、根本的な解決にはならないため、Rails側には入れたくないということのようである。

それはもっともな話だと思うのだけれども、複数ユーザからの同時更新が仕様的に通常は走らないような場合、基本的にはRails側でユニーク性を保証しつつ、万が一の場合にだけDBのユニーク制約に引っかかることによるInternal Server Errorでいいやというニーズはけっこうあるのではと思う。ちょっとサボりすぎかな。何か勘違いなどしていたらごめんなさい。

結局、以下のような対応をした。メモリ上でユニークかをチェックするようにした。
出典URL:http://stackoverflow.com/questions/2772236/validates-uniqueness-of-in-destroyed-nested-model-rails

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
class Author
has_many :books

# Could easily be made a validation-style class method of course
validate :validate_unique_books

def validate_unique_books
validate_uniqueness_of_in_memory(
books, [:title, :isbn], 'Duplicate book.')
end
end

module ActiveRecord
class Base
# Validate that the the objects in +collection+ are unique
# when compared against all their non-blank +attrs+. If not
# add +message+ to the base errors.
def validate_uniqueness_of_in_memory(collection, attrs, message)
hashes = collection.inject({}) do |hash, record|
key = attrs.map {|a| record.send(a).to_s }.join
if key.blank? || record.marked_for_destruction?
key = record.object_id
end
hash[key] = record unless hash[key]
hash
end
if collection.length > hashes.length
self.errors.add(:base, message)
end
end
end
end

以上、まとまりのない話になってしまいましたが、本年もどうぞよろしくお願い申し上げます。

このエントリーをはてなブックマークに追加