🏠

続・マチマチとOSSの関わりをGitHubで振り返る

2019/03/06に公開

Rails

ご近所SNS「マチマチ」を作っている武者(@knu)です。前回に引き続き、今回もマチマチとOSSとの関わりについて紹介したいと思います。

不具合のフィードバック

それなりの規模のプロダクトを開発していれば、当然さまざまなライブラリやツールを使うことになりますが、自分達では特に変わった使い方をしているつもりはないのに、ふとバグに当たることがたびたびあります。認知バイアスの可能性もありますが、しょっちゅう 「なぜ我々が第一発見者なのか？みんな踏んでいないの？」 と感じている気がします。

そこで思い至るのは、「バグをバグであると認知して指摘する」こと自体が、スキルや経験の裏打ちを必要とする所業なのかもしれないということです。

自分に自信がない、ちゃんと調べる気に至らない、などの引っ込み思案に陥り、「きっと自分の使い方が悪いのだろう」「回避したら動いたからよしとしよう」と撤退してしまうエンジニアが思った以上にたくさんいます。明らかに想定外の使い方をしていながらバグだと思い込んで騒ぎ立てるような人もいるにはいますが、もっと恐れずにフィードバックをしていいのにと思います。

「こういう使い方ができると思った」「そこでこう書いた」「しかし結果は期待と違ってこうだった」という筋道立った報告は、それがバグでなく仕様だったとしても、開発者にとって貴重でありがたいフィードバックになっていることも多いです。直接的にはドキュメントの充実、中長期的には勘違いの起こりにくいAPIを設計するための参考になります。

というわけで張り切ってバグ報告していきましょう！

PDFのサムネール取得

マチマチでは、自治体の配布するチラシや広報のサムネイル画像を作るためにgrimというgemを使っています。特定のファイルをアップロードするとエラーになる事象があり、調べたところ、ファイル名に半角括弧「(」が含まれるとエラーが出ると分かりました。恐る恐るgemのソースを確認すると、見事なエスケープ漏れでした。（つまり脆弱性…😱）

shellescape は私が実装したメソッドだけに、始めからちゃんと使ってほしかった！と思いました。最小のパッチで投げてありますが、本当は引数列を shelljoin で結合する形にしたいところです。

PRを出したときは、マージされるまでは一時的にgithub sourceでforkを使い、マージ後に新しいバージョンが出たらsourceを戻してbundle update {gem名}する、という手順になります。

日本の電話番号

マチマチでは、ユーザのSMS認証やローカルプレイスデータの整備で数多くの電話番号を取り扱います。電話番号のバリデーションや整形表示（適切な桁数で区切りを入れる）など便利な機能を提供してくれるのが、phonyというgemです。

このライブラリが0800で始まるフリーダイヤルの番号を正しく扱えなかったため、調べたところ、データに誤りがあると分かりました。0800の後には6桁ではなく、7桁の番号が続きます。（0120より1桁多い）

PRを出し、正規の出典（現総務省サイトにある当時の郵政省のプレスリリースのURL）も示して解説を添えました。 マージの判断に必要な情報を提供する のも、素早くマージしてもらうためには大事なことです。

ActiveSupportのおせっかいに潜むバグ

RubyにはBourne shell, Perlの伝統を引き継いだシェルコマンド展開 `…` が実装され、 Kernel#` メソッドを通じて提供されています。

このメソッドは、コマンドが存在しないなどの場合には Errno::ENOENT 例外が発生しますが、それ以外はコマンド自体がエラー終了した場合でも必ず出力文字列を返すはずです。ところが、 rails runner を通じて実行したスクリプトの内部の `…`.chomp の部分で、なぜか NoMethodError (undefined method`chomp' for nil:NilClass) で落ちているのが発見されました。

ありえない…と、調査したところ、なんとActiveSupportが Kernel#` をオーバーライドしており、その中の処理に問題があることが分かりました。

コマンドが存在しないときに Errno::ENOENT を握り潰して、標準エラー出力にその旨を表示しています。悪いことに、 rescue 節が STDERR.puts の呼び出しのまま終わっているため、 IO#puts の返り値であるnilが返却されてしまうというわけです。1

背景としては、Unixではメタ文字を含む場合はシェルを介し、含まない場合直接 execve(2) するようになっており、コマンドが存在しない場合の挙動がシェルを介するかどうかで変わってしまうことから、与えた文字列やプラットフォームの事情に依らず同じ動きに揃えたかったということのようです。

私の最初の提案は、握り潰すのであれば空文字列("")を返すようにしよう、というものでしたが、最終的には歴史的経緯はともあれRuby本来の挙動のままでいいだろう、と削除されることになりました。

これは合意が取れてからもかなりマージに時間が掛かりました…。

`…`の未来

なお、これとは別にMatzから出た話として、 `…` は将来的に廃止して ` を別の意味に使いたい、というアイデアがあります。標準出力はキャプチャするがデフォルトでエラー出力をstderrに出してしまう Kernel#` の挙動がいまいちである、などの理由付けもありますが、Rubyの文法を拡張したい何かが念頭にあるようです。（「ネームスペース」というワードは出ていますが実体はまだ謎に包まれています）

昨今はRubyの用法の多くがWebプログラミングの文脈となり、昔ほどシェルコマンド展開を使わなくなったということもあるでしょうか。そもそもオリジナルのBourne shellでは $(…) という代替記法の方がどちらかといえばメジャーですね。

また、Go, Haskell, ECMAScriptなどでそれぞれバッククオートが有効活用されている様子は、ASCII記号の枯渇と戦っているRubyからは魅力的に思えます。

どうなるかは分かりませんが、現在の `…` が廃止された場合も %x[…] 記法は残されると思うので、僅かな変更（従来の `…` の頭に %x を付ける）で従来のコードを動かすことはできそうです。（自動修正も可能でしょう）

ActiveRecordのバグ

ActiveRecordには、巨大な結果セットを OFFSET を進めながら LIMIT で少量ずつ取ってくれるfind_each / find_in_batches という便利なメソッド（ActiveRecord::Batchesモジュールが提供）がありますが、これはプライマリーキーの昇順でしか行えないので、時には手で OFFSET / LIMIT を指定してループしたいことがあります。

relation = Places
  # …
  .joins(city: :prefecture)
  .select("places.*")
  .select("cities.code")
  .order(Arel.sql("cities.code ASC"))
  .order(:id)
  .distinct
    
batch_size = 100
    
0.step(by: batch_size) do |offset|
  records = relation.offset(offset).limit(batch_size)
  break if places.empty?
    
  # placesを使う
end

こんなコードを書いていたのですが、なぜか、 break if places.empty? で抜けず下に進んだのに、 places を使おうとすると中身は空、という事象が発生していました。

調べてみたところ、ActiveRecord::Relationが empty? / exists? でレコードの存在を安価なコストでチェックするために SELECT 1 AS one … LIMIT 1 というSQLを発行しており、これが DISTINCT と OFFSET を併用した際には実体と食い違う結果を招いていることが原因でした。

このSQLには DISTINCT が付いていないため、重複レコードによる水増しでやっと OFFSET を超えるようなケースでは、 DISTINCT の下では取れないのに「取れる」と誤判定してしまうわけです。

ここの修正は職人芸が要るな、と思ったので、PRでなく再現ケースを書いて単にissueを上げたところ、すぐにkamipoさんが現れて直してくれました。

Railsの新しいリリースまでには間があるので、 .to_a を付けてRelationを実体化（配列化）してしまうことでworkaroundとしています。

Cloudflareのキャッシュパージ

マチマチではCDNとして主にCloudflareを使っています。メディアサイトの本格始動以降、コンテンツの拡充とともにUUもPVもまさに桁違いに激増し、今も急成長しているため、CDNのエッジキャッシュで多大な恩恵を受けています。

しかし、キャッシュが難しいのはみなさんご承知の通りです。データの変化と連動して適宜関連ページのキャッシュをパージすることで、コンテンツの更新がいち早くユーザに届くようにする必要があります。キャッシュのパージにはAPIを用いるのが通例です。

CloudflareのAPIはシンプルなので、ふつうのHTTPクライアントライブラリを使っても十分ですが、せっかく専用ライブラリがあるのなら、使いやすさや保守性を求めて乗っかりたいところです。

というわけで、いくつか比較検討した結果、まずはcloudflairというgemを使い始めました。なお、最初はキャッシュパージではなくDNSレコードの操作のために使っていました。ところが、キャッシュパージAPIを同ライブラリから呼び出そうとしたところ、エラーが返ってきます…。見てみると、 POSTを発行すべきところ、DELETEを発行 していました。キャッシュを消す、というイメージに引っぱられてしまったのでしょうか…。

よく見ると採用後にコミットがないし、アクティブではなさそう。キャッシュパージをするユーザがいなかったので発覚せず、結局そのまま静かに眠りに就いてしまうのでしょうか…。

やむを得ず、前回比較した際の対抗馬だったcloudflare gemへの移行を進めました。独特の非同期フレームワーク（async）をバリバリに採用しており、そこまで性能を重視しない身からすると余計な面倒は避けたい、という心情から前回の選定では見送ったのですが、こちらの方が活発に開発が進行しており、 賭けに負けた 敗北感を味わうことになりました。

無事、DNS部分の移行はすんなり行ったので、「ああよかった、ではキャッシュパージを…」と進んだところで、再び愕然とすることになります。なんと、このgemもキャッシュパージAPIの実装に誤りがあったのです。コードの意図としてはパス名に指定しているつもりの purge_cache が、キーワード引数のミスマッチにより、 あろうことかURLパラメータ ?path=purge_cache に化けてしまっている…。 さっそく修正、テストしてPRを出しました。

こちらは執筆時点で取り込み待ちですが、いずれマージされると思います。

結局、 「誰もキャッシュパージにgemを使っていなかった」 が結論ということでよろしいかと思います。なんてこった…。😩

機能拡張のフィードバック

差分データの展開

2つのデータの並びを比較して、差分を計算するdiff-lcsというgemがあります。2つのファイル（バージョン）の内容の行単位の差分を取る diff(1) で使われているMcIlroy-Hunt longest common subsequenceアルゴリズムをライブラリ化したものです。

マチマチでは、一部コンテンツの差分更新を行う処理でこれを用いています。開発時、まずREPL (irb/pry)でDiff::LCSの挙動を軽く確認した上で、実際のコードに落としていきました。REPLでの表示の様子から、てっきり差分リストの各要素は配列だと思い、以下のようなコードでブロック引数への多重代入で受けようとしたのですがなぜかうまく動きません。

Diff::LCS.sdiff(a, b).each do |action, (old_position, old_element), (new_position, new_element)|
  case action
  when '!'
    # replace
  when '-'
    # delete
  when '+'
    # insert
  end
end

diff-lcsの実装を見たら、このオブジェクトは配列ではなく、配列っぽい文字列表現を採用していただけでした。REPLでの見た目にだまされた…。

オブジェクトを単値で受けて .to_a してやれば分解できるのですが、それはあまりに面倒だと思ったので、脊髄反射的に to_ary を実装するだけのPRを出しました。

CIがこけたら直そう、と気軽に出したのですが、あっさり通ってしまい、ほどなくマージされます…。

ところが、改めて手元でテストを動かしてみると大量にこける…。中を見ると、差分リストを flatten しているところがあり、 to_ary があるために、今まで保存されていた各要素の中身もすべてflattenされてしまったというわけです。^[1]

作者の方もこれに気づき、revertした上で改めてflattenしていたところを書き直してくれたので一件落着です。なお、CIは設定が壊れていてテストが走らずに通ってしまっていたようです。

メソッドが一つ増えただけなのに壊れてしまう、というやや珍しい例で、次回からは気を付けようと思います。😅

Bulk InsertをPostGISでも動かす

マチマチでは、各地域情報の充実のため、大規模なデータ投入を定常的に行っています。まとまったデータを投入する際、1レコードずつINSERTしていては性能が出ないので、個人的に利用経験のあったbulk_insert gemを使おうと思いました。

ちなみにactiverecord-importというgemもあり、実際これを使ったこともあります。こちらはActiveRecord親和的なAPIで、オプショナルですがvalidationを行わせることもできます。ただ、モデルのデータ構造を介してしまう分オーバーヘッドがあります。

bulk_insertの方は、生のINSERT文を発行することに特化しており軽量です。自分でループを回してbulk_insertに食わせていくと、適当な数（デフォルトでは500）ごとに自動でINSERTを発行してくれるのが便利です。今回は、データの正しさはあらかじめ確認できているためこちらを使うことにしました。

そういえば、Rails 6にbulk insert機能が入るかどうか、ちょっと注目ですね。☝️

さて、このbulk_insertを使おうとしたところ、 ignore: true モード（重複は無視＝スキップ）がactiverecord-postgis-adapterに対応していないことが分かりました。

見てみると、PostGISアダプターだけでなく、少なくとも日本ではいちばんユーザが多そうな Mysql2 アダプターや、私が前職でも使っていたactiverecord-mysql2spatial-adapterにも対応していないようだったので、ついでに対応させるPRを出しました。

昨日の自分は明日の誰か、今日の誰かは明日の自分。みんなつながっています。😇🤝😇

開発便利ツールの公開

開発に使う道具も、みんなで共有するメリットはたくさんあります。いいものを作って気に入っていても、それにばかり手をかけるわけにはいかないことも多いです。となれば、公開してユーザが集めるのがリスクと手間を減らすことにつながります。周辺の状況が変化して有用性が失われそうになったときに、自分の代わりに修繕・改善してくれる人が現れてくれたらうれしいですね。

RSpecのマッチャー

WebのE2Eテストでは、UIを中心に非同期処理が多いため、そうした面でRSpecのマッチャーの支援を期待したいところです。

幸い、Capybaraの have_content とか have_current_path などのマッチャーは所定のタイムアウトまでに条件が成立するかをポーリングしてチェックしてくれます。では、サーバサイドのデータの状態が非同期に更新される場合の検査についてはどうか。

…残念ながら見当たらなかったので、所定の時間内に条件が成立するかを見る become マッチャーを作って使っています。

マチマチにおけるテストの方針についてはこちらの記事もどうぞ。

db/structure.sql の自動マージ

マチマチでは、各種拡張（extensions）、制約（constraints）、トリガー（triggers）、 ON DELETE 節等々、PostgreSQL/PostGISのさまざまな機能を使い尽くしているので、スキーマを db/schema.rb で表現することはとうにあきらめています。(config.active_record.schema_format = :sql)

そこで問題になるのが、「スキーマ変更が交錯すると毎回必ずconflictが起きる」ことです。というのも、 db/structure.sql はほぼ生のSQLダンプファイルで、末尾はこのように schema_migrations テーブルへのINSERT文になっています。

INSERT INTO "schema_migrations" (version) VALUES
-- …
('20190219072917'),
('20190219084823'),
('20190220030558');

これでお察しの通り、途中まで , で最後だけ ; が付くため、この後に何か追加されると

-('20190220030558');
+('20190220030558'),
+('20190221012345');

のように行の追加だけでなく変更が発生してしまい、必然的に複数の差分が重なると自動マージが働きません。（仮に記号の問題がなくても、変更箇所が重なると自動マージは無理ですね…）

これは色々な機能を並行開発している際にかなり辛いので、 db/structure.sql 専用のGitのマージドライバーを書きました。

例によって、ついでにMySQLやSQLite3にも対応させてあります。

ドキュメントの通り、インストールは簡単にしてあるので、同じ境遇の方はぜひご利用ください。

Ridgepoleだとスキーマバージョンというものはないためこのマージ問題はなさそうですが、マージ問題だけで db/structure.sql をやめてRidgepoleを使えないか検討している、といった向きにはこのツールは朗報かもしれません。

このマージドライバーは、構造上差分が生じてしまう schema_migrations などの部分だけ自力マージして、後はGitのデフォルトマージドライバーである git merge-file に任せる実装になっています。

スクリーンビデオキャプチャのGIF化

表示/UI系のissueやPRを出すときは、スクリーンショットを付けないと何のこっちゃとなりがちです。ところが、GitHubには動画をインラインで貼ることができません。唯一貼れるアニメーションGIFも、ファイルフォーマットの制約で256色しか使えません。

<FONT COLOR="#ffcc00">…</FONT> （Webセーフカラー216色を意識）で表示もビットマップフォント、なんて時代から過ごしてきた身からすると、 「よほどきれいな写真でも入っていない限り、256色もあれば行けるんじゃね？」 とつい思ってしまうのですが、流麗なフォントスムージング、丸みを得た枠線、アルファチャンネルによる半透過効果などなど、今やごくシンプルに見えるページでさえレンダリングには数十のオーダーで色が使われるようになっています。パレットの最適化を行わずに素朴にffmpegなどで変換すると、ディザリングの嵐となってまだら模様が目立ってしまい、とても見られた動画にはなりません。

何かいいツールはないか、と思ったら、こんな記事を見つけました。

ffmpegのパレット生成機能を使い、2パスで変換すると、きれいにエンコードできるというお役立ち情報です。これは良いことを知りました。

ということで、シェルスクリプトを書くだけでも良かったのですが、MacのUIから楽に呼び出せるようにAppleScriptでくるんでアプリにしてみました。

https://github.com/knu/mov2gif-for-mac

ところで、AppleScriptってどうやって体系的に学ぶんでしょうね。私は毎回ググっては切り貼りして、試行錯誤で七転八倒しています。

Markdownでチェックボックス付きリストを使う

esaやGitHubのissue/PRでチェックボックス付きリストを書くことが多いので、Emacsの markdown-mode にチェックボックスを入れる機能を追加しました。

ブラウザ上のテキストエリアも、GhostText（Chrome, Firefox）を使えば好きな外部エディタで編集できます。

巨大YAMLの編集で迷子にならない

Webの開発をしていると、なぜか巨大なYAMLファイルと格闘するはめになることが多いですね。

RailsでもメッセージカタログはYAML形式だし、CIの設定ファイルもYAMLで、最初はシンプルでもいつのまにかビルド手順やマトリックスが複雑になって肥大化していきます。JSONと違ってコメントも書けるけれど、そもそも今自分が見ている行はどこなんだろう…という問いから逃れるのは至難です。

私はEmacsを使っているので、 which-function-mode および imenu という仕組みを使って今いる行のYAMLパスを常時表示できるパッケージを書きました。階層を選んでジャンプもできます。

https://github.com/knu/yaml-imenu.el

おわりに

今回は少し長くなってしまいました。事の経緯をトラブル発生から描くと、一つ一つが物語になってしまいますね。

日々直接携わっているプロダクト・サービス自体以外にも、この事業をしていなければこの世にこのタイミングで起きなかったかもしれないことがいろいろあるなあ、と振り返ることができました。

自分達が困ったから直す、あるいは作る、が基本ではありますが、きっと誰かの役にも立つはず、という思いで一つ一つの問題に取り組むことは心を豊かにしてくれます。

~~引き続きマチマチでは、OSSで最高にハイになりたいエンジニアを募集しています。武者にお気軽にお声掛けください！~~

脚注

冒頭の話と絡めると、返り値がnilだと、「コマンドが存在しないときはそういうこともあるか」と妙に納得してしまい、自分のコードを修正してしまうユーザもそれなりに多そうです。 ↩︎