ActiveDecoratorにプルリクを投げるまでにやったこと

怪しい挙動を見つけたきっかけ

テストコードで以下のようなことをしてました。

Rails.application.eager_load!

testでもproductionと同じ環境になるように事前に読み込んでいたのですが、テストを個別に実行するときにも全ファイルを読み込んでしまうため消したいと思っていました。

試しに消してCIを回してみると、一部のテストがfailした。

どんな問題が起きたか？

再現するコード例は以下の通り。

form = Form::User.new
ActiveDecorator::Decorator.instance.decorate(form)
form.is_a?(UserDecorator)  #=> true

Form::UserDecorator を定義してないときに、なぜか UserDecorator が使われてしまう。

コードを読んで発生箇所を突き止める

ActiveDecorator のコード内で Object.const_get を使っている箇所が UserDecorator を返していた。

• lib/active_decorator/decorator.rb#L72

そして、モジュールの自動読み込みに関する処理は ActiveSupport に定義されている。

• active_support/dependencies.rb

ここまで調べて、自動読み込み周りはどう直すべきかよく分からなかった。

OSSパッチ会で松田さんに聞く

ちょうど問題を見つけた2日後にパッチ会があったので、直接聞いてみることにした。

blog.agile.esm.co.jp

a_matsuda さんに聞いてみたところ、以下のことが分かった。

• 最近、そのあたりのコードを変えた
  • v1.2.0、v1.3.0（最新）で挙動を確認してみて欲しい
• 意図した挙動ではない（バグっぽい）

動作確認してみると、 v1.2.0 では問題が起きず v1.3.0 で問題が起きること が判明した！

再現するテストケースを書く

v1.2.0 では問題ない事が分かったら、すぐ amatsuda/active_decorator@73dcdd6 が原因だと分かった。

問題を再現させるテストケースを書き、意図通りに失敗させて、 73dcdd6 をリバートしてテストが通ることを確認した。

英語のコミットログを書く

自分が最初に書いた英語のコミットログがこれ。

Fix a wrong decoration for nested classes

There is a problem that unintended decorators are used for nested classes.
This commit reverts 73dcdd67ae146ee9e76b1da9ae9e9dbd55529193 to fix the problem.

英語が苦手なので、OSSパッチ会に参加していた @yahonda さんにレビューしてもらいました。フィードバックは以下の通り。

• wrong や unintended が何か分からない
  • actual と expected を書いた方が良い
• 細かいけど a wrong decoration の a は要らない
• unintended より unexpected の方が良い

レビュー頂いて気づいたけど、英語の問題の前にコミットログの内容が足りていなかった...。

直したコミットログ

どういう問題が起きていて、何を期待しているかが分かるようにコミットログを直してプルリクを作成した。

Fix wrong decoration for nested classes

ActiveDecorator decorates incorrectly for the nested class that do not define decorator.

comic = Foo::Comic.new
ActiveDecorator::Decorator.instance.decorate(comic)
comic.is_a?(ComicDecorator)  #=> true

If `Foo::ComicDecorator` is not defined, it should not use `ComicDecorator` instead of `Foo::ComicDecorator`.
The cause of the problem is 73dcdd6, so we will revert it.

github.com

まとめ

OSSパッチ会のおかげでプルリク作るところまで出来た。ありがたい。

ActiveRecordのissueで再現コードを書くときに発行されたSQLを確認する方法

rails/rails#36895 の挙動を確認してみようかなーと思ったけど、発行されるSQLを確認する方法が分からなかったので調べた。

Rails本体のテストコードだと assert_no_queries が使えるけど、これは外で使えないので。

動いたコード

assert_no_queriesの定義されている場所を git grep 'def assert_no_queries' すると、実装はrails/activerecord/test/cases/test_case.rb に書かれていた。

必要なのはSQLCounterのクラスぐらいなので、これを再現コードにコピペする。

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"
  gem "rails", "5.2.3"
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

puts
puts "Ruby = #{RUBY_VERSION}"
puts "ActiveRecord.version = #{ActiveRecord.version}"
puts

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table(:users, force: true)
  create_table(:posts, force: true) do |t|
    t.references :user
    t.timestamps
  end
end

class User < ActiveRecord::Base
  has_many :posts
  has_many :new_posts,
    -> { where("posts.created_at > ?", 1.week.ago) },
   class_name: "Post"
end

class Post < ActiveRecord::Base
  belongs_to :user
end

require "active_support/notifications"

# refs: activerecord/test/cases/test_case.rb
class SQLCounter
  class << self
    attr_accessor :ignored_sql, :log, :log_all
    def clear_log; self.log = []; self.log_all = []; end
  end

  clear_log

  self.ignored_sql = [/^PRAGMA/, /^SELECT currval/, /^SELECT CAST/, /^SELECT @@IDENTITY/, /^SELECT @@ROWCOUNT/, /^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/, /^SHOW max_identifier_length/, /^BEGIN/, /^COMMIT/]

  # FIXME: this needs to be refactored so specific database can add their own
  # ignored SQL, or better yet, use a different notification for the queries
  # instead examining the SQL content.
  oracle_ignored     = [/^select .*nextval/i, /^SAVEPOINT/, /^ROLLBACK TO/, /^\s*select .* from all_triggers/im, /^\s*select .* from all_constraints/im, /^\s*select .* from all_tab_cols/im, /^\s*select .* from all_sequences/im]
  mysql_ignored      = [/^SHOW FULL TABLES/i, /^SHOW FULL FIELDS/, /^SHOW CREATE TABLE /i, /^SHOW VARIABLES /, /^\s*SELECT (?:column_name|table_name)\b.*\bFROM information_schema\.(?:key_column_usage|tables)\b/im]
  postgresql_ignored = [/^\s*select\b.*\bfrom\b.*pg_namespace\b/im, /^\s*select tablename\b.*from pg_tables\b/im, /^\s*select\b.*\battname\b.*\bfrom\b.*\bpg_attribute\b/im, /^SHOW search_path/i, /^\s*SELECT\b.*::regtype::oid\b/im]
  sqlite3_ignored =    [/^\s*SELECT name\b.*\bFROM sqlite_master/im, /^\s*SELECT sql\b.*\bFROM sqlite_master/im]

  [oracle_ignored, mysql_ignored, postgresql_ignored, sqlite3_ignored].each do |db_ignored_sql|
    ignored_sql.concat db_ignored_sql
  end

  attr_reader :ignore

  def initialize(ignore = Regexp.union(self.class.ignored_sql))
    @ignore = ignore
  end

  def call(name, start, finish, message_id, values)
    return if values[:cached]

    sql = values[:sql]
    self.class.log_all << sql
    self.class.log << sql unless ignore.match?(sql)
  end
end

ActiveSupport::Notifications.subscribe("sql.active_record", SQLCounter.new)

def capture_sql
  SQLCounter.clear_log
  yield
  SQLCounter.log.dup
end

require "active_support/testing/time_helpers"

class BugTest < Minitest::Test
  include ActiveSupport::Testing::TimeHelpers

  def test_to_reproduce
    travel_to Time.new(2019, 8, 10)
    User.create

    log = capture_sql { User.includes(:new_posts).to_a }
    assert_equal 'SELECT "users".* FROM "users"', log[0]
    assert_equal 'SELECT "posts".* FROM "posts" WHERE (posts.created_at > \'2019-08-02 15:00:00\') AND "posts"."user_id" = ?', log[1]
  end
end

includes に関する挙動は to_sql だと確認できないので、そういうバグを踏んだときに使えるようにメモしておく。

Railsのissueに書いてある再現コードを使ってgit bisectする方法

備忘録。

具体的な例

先日に登録されたissueを例にしてみる。

github.com

再現コードを少し書き換える

v5.2.3と6.0.0.rc2でsqlite3の依存周りでエラーが出たので、Gemfileのあたりを少しだけ弄った。

# frozen_string_literal: true

require "bundler/inline"

gemfile = File.read(File.expand_path("Gemfile", __dir__)).strip
version = gemfile.lines.find { |line| line.include?('sqlite3') }
  .then { |sqlite3_line| sqlite3_line.strip.split(',').last.delete('"') }

gemfile(true) do
  source "https://rubygems.org"
  gem "rails", path: "."
  gem 'sqlite3', version
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :articles, force: true do |t|
    t.string :title
    t.string :category
  end
end

class Article < ActiveRecord::Base
end

puts
puts "ActiveRecord.version = #{ActiveRecord.version}"
puts

class BugTest < Minitest::Test
  def test_to_group_by_count_with_order
    article = Article.create(title: 'foo', category: 'foo')

    assert_equal ({"foo"=>1}), Article.group(:category).order('count_articles_id').count('`articles`.`id`')
  end
end

実際の手順

$ git clone https://github.com/rails/rails.git
$ cd rails
$ vim
(上記の再現コードを issue.rb として保存）
$ git bisect start v6.0.0.rc2 v5.2.3
$ git bisect run ruby issue.rb
.
.
.
(コーヒーでも飲みながら待つ）
.
.
.
Finished in 0.008043s, 124.3317 runs/s, 0.0000 assertions/s.
1 runs, 0 assertions, 0 failures, 1 errors, 0 skips
c9e4c848eeeb8999b778fa1ae52185ca5537fffe is the first bad commit
commit c9e4c848eeeb8999b778fa1ae52185ca5537fffe
Author: Ryuta Kamizono <kamipo@gmail.com>
Date:   Sat Apr 6 13:04:06 2019 +0900

    Don't repeat same expression in SELECT and GROUP BY clauses

    This refactors `execute_grouped_calculation` and slightly changes
    generated GROUP BY queries, since I'd not prefer to repeat same
    expression in SELECT and GROUP BY clauses.

    Before:

    ```
    SELECT COUNT(*) AS count_all, "topics"."author_name" AS topics_author_name, COALESCE(type, title) AS coalesce_type_title FROM "topics" GROUP BY "topics"."author_name", COALESCE(type, title)
    ```

    After:

    ```
    SELECT COUNT(*) AS count_all, "topics"."author_name" AS topics_author_name, COALESCE(type, title) AS coalesce_type_title FROM "topics" GROUP BY topics_author_name, coalesce_type_title
    ```

    Although we generally don't guarantee to support Arel node constructed
    by user itself, this also fixes #24207.

 .../lib/active_record/relation/calculations.rb     | 46 ++++++++++------------
 1 file changed, 20 insertions(+), 26 deletions(-)
bisect run success

なるほど、こうやってコミットを探すのか。

追記

bisect が終わった後に元のブランチに戻るには git bisect reset を実行する。

minitestで書かれたActiveRecordのテストで特定のケースだけを実行する方法

追記: activerecord/RUNNING_UNIT_TESTS に書いてあった。

---

自分用の備忘録。

特定のファイルを実行する方法

$ cd activerecord
$ ARCONN=postgresql ruby \
  -I"lib:test" \
  test/cases/adapters/postgresql/range_test.rb

特定のテストケースを実行する方法

$ cd activerecord
$ ARCONN=postgresql ruby \
  -I"lib:test" \
  test/cases/adapters/postgresql/range_test.rb \
  -n test_endless_range_values

DMMを退職しました

退職エントリを書くつもりはあまり無かったけど、はてブに退職エントリが多くあがっていて書く気力が高まったので書いてみた。

DMMに入社した理由

去年の5月に入社しました。

• 大きな企業の便利な社内ツール・サービスを見たい
• 色んな種類のサービスの裏側を見たい
• Railsアプリをゼロから作る機会
  • 仕事で rails new できる
  • プロジェクト初期からレビューできる

あたりに魅力を感じて内定を承諾した。

DMMでやっていたこと

とあるサービスのリプレイス案件*1のサーバサイドエンジニアを担当していた。

• 既存のデプロイ方法の修正
• AWSのアーキテクチャ設計
• APIや管理画面をRailsで実装
• RubyやRailsに関するレビュー（他メンバーがあまりRailsに慣れていなかった）
• Terraformを使ったインフラ業
• 死活監視の設定
• タスク管理

この案件、入社して2ヶ月でデスマの気配を感じていた。

自分の予知能力Lv3だと死ぬ未来が見えてるんだけど、他メンバー全員が合意してるし、きっとみんなの予知能力が高くてプロジェクトが成功する未来が見えているのだろう。信じる。

— 神速 (@sinsoku_listy) 2018年6月18日

そして、9月くらいにデスマを避けられないと感じていた。

何をやっても世界線が収束してデスマになりそうな気がするので、諦めた方が早いのでは…。

— 神速 (@sinsoku_listy) 2018年9月3日

リリース前は死ぬほど大変だったけど、なんとか生きてリリース日を乗り越えることが出来て良かった。
デスマを予測できていたのに、どうにも出来なくて自分のスキル不足を痛感した案件だった。

プロジェクト管理に関しては 言いたい事が山程ある けど、これは綺麗に書ける気がしないので控えておく。*2

リリース後

リリース後は落ち着いたのでふりかえりをしたり、開発の進め方を変えたりしていた。 そして、役割がスクラムマスターになったのでスクラムマスター業をしていた。

あと、隙間時間で他チームのRailsアプリのレビューを勝手にしていた。 パフォーマンス改善や脆弱性を指摘できたので、少しは貢献できていたと思う。

辞めた理由

色々とあるけど、あまり書くと残っている方々に迷惑がかかるので、当たり障りのない範囲だけ書いておく。

• 自分のやりたい事は終わった
  • 大きな企業の社内サービス、ツールの状況がわかった
  • Railsでゼロからアプリを作ってリリースまで終わった
• 査定結果が年収640万だった
  • 年収交渉したが、事業部で昇給するのは無理だった
  • 異動を含めた交渉もしたが、無理だった
• DMMに残りたい理由が特になかった

DMMは売上のある事業をいくつも持っているし、多くのエンジニアを抱えていてポテンシャルは高いと思う。 ただ、制度や組織体制には微妙な点（=改善できそうな点）が非常に多い。

こういう点を直すのには興味あって、横断的な部署に異動して技術で会社を改善していきたいな...と半年前くらいから1on1で異動の打診はしていた。 ただ、需要がないのか、自分の技術不足なのか無理だった。

それ以外で特にDMMでやりたい事もなかったので、退職することにした。

次にやること

去年の開発で疲れたので、当分はフリーランスで週1だけ仕事して、残りはOSSにパッチでも投げる生活をしようかなと思っています。

あと、作ってみたい個人サービスのネタがあるので、プロトタイプを作ってみる予定。

*1:残念ながらアダルト関連ではない

*2:興味ある人はオフラインで聞いてください

ActiveRecordでRails.cacheを良い感じに使うgemを作った

作ったgemはこれです。

github.com

使い方

ApplicationRecord でモジュールを include してください。

class ApplicationRecord < ActiveRecord::Base
  include ActiveRecordInCache::Methods

  self.abstract_class = true
end

自動的に maximum(:updated_at) を使ったキャッシュキーを生成し、2回目以降は Rails.cache からオブジェクトを取り出します。

Article.all.in_cache
# SELECT MAX("articles"."updated_at") FROM "articles"
# SELECT "articles".* FROM "articles"
#=> [#<Article:0x0000000000000000>, ...]

Article.all.in_cache
# SELECT MAX("articles"."updated_at") FROM "articles"
#=> [#<Article:0x0000000000000000>, ...]

実装について

中身は3行しかありません。

def in_cache(column = :updated_at, options = {}, &block)
  value = block_given? ? all.instance_exec(&block) : all.maximum(column)
  name = "#{all.to_sql}_#{value}"
  Rails.cache.fetch(name, options) { all.to_a }
end

キャッシュのキーは #{sql}_#{maximum_updated_at} になります。

あと、ブロックを使えるようにしてあるので、maximum 以外の処理にしたり、suffixをつけることができます。

Article.all.in_cache { "#{maximum(:updated_at)}_v2" }

Puppeteerを使ってDMMの同人ランキングから人気のあるジャンルを列挙してみる

とりあえず、Puppeteerを使ったE2Eテストをスラスラ書けるようにするため、DMMやとらのあな、メロンブックス辺りをスクレイピングして、人気の性癖を調べる実用的なコードでも。

— 神速 (@sinsoku_listy) 2019年6月10日

E2Eテストの練習としてPuppeteerで実用的なコードを書いてみた。

やったこと

FANZA同人のランキング1〜100位の作品を取得し、全てのジャンルを計測して 人気ジャンル を出してみた。

www.dmm.co.jp

ソースコード

const puppeteer = require('puppeteer');
let browser;
beforeEach(async () => browser = await puppeteer.launch());
afterEach(async () => await browser.close());

const timeout = 20 * 60 * 1000; // 20 minutes

displayRanking = async (page) => {
  for(let i = 1; i < 5; i++) {
    await page.evaluate(_ => window.scrollBy(0, document.body.scrollHeight));
    await page.waitFor(`.rank-rankListItem:nth-child(${20 * (i + 1)})`);
  }
};

findTags = async (page, urls) => {
  const tagsArray = [];

  for(let i = 0; i < urls.length; i++) {
    console.log(`${i}: parsing... ${urls[i]}`);

    await page.goto(urls[i]);
    const tags = await page.$$eval('.genreTagList__item', divs => divs.map(div => div.innerText));
    tagsArray.push(tags);
  }

  return tagsArray.flat();
};

test('doujin', async () => {
  const page = await browser.newPage();
  await page.goto('https://www.dmm.co.jp/dc/doujin/-/ranking-all/=/sort=popular/term=monthly/');

  const title = await page.$eval('.c_hdg_withSortTitle', el => el.innerText);
  expect(title).toMatch(/総合ランキング 月間/);

  await displayRanking(page);
  const itemsCount = await page.$$eval('.rank-rankListItem', items => items.length);
  expect(itemsCount).toBe(100);

  const urls = await page.$$eval('.rank-name a', links => links.map(link => link.href));
  const tags = await findTags(page, urls);

  const ranks = {};
  tags.forEach((tag) => {
    if (ranks[tag] === undefined) ranks[tag] = 0;
    ranks[tag] += 1;
  });
  ranks['成人向け'] = 0;
  ranks['男性向け'] = 0;
  const topRanks = Object.entries(ranks).sort((a, b) => b[1] - a[1]).slice(0, 20);

  console.log(topRanks);
}, timeout);

無限スクロール

最初は1〜20位までしか表示されていないので、スクロールしつつ waitFor で作品の表示を待つようにする。

displayRanking = async (page) => {
  for(let i = 1; i < 5; i++) {
    await page.evaluate(_ => window.scrollBy(0, document.body.scrollHeight));
    await page.waitFor(`.rank-rankListItem:nth-child(${20 * (i + 1)})`);
  }
};

各ページのタグ取得

タブを生成して、並列に処理しようとしたら TimeoutError: Navigation Timeout Exceeded: 30000ms exceeded が起きて、解決方法が分からなかった。

とりあえず、1ページずつ順番に処理する方法で対応。（時間かかるけど）

findTags = async (page, urls) => {
  const tagsArray = [];

  for(let i = 0; i < urls.length; i++) {
    console.log(`${i}: parsing... ${urls[i]}`);

    await page.goto(urls[i]);
    const tags = await page.$$eval('.genreTagList__item', divs => divs.map(div => div.innerText));
    tagsArray.push(tags);
  }

  return tagsArray.flat();
};

ランキング計測

Rubyのgroup_byみたいなのが見当たらなかったので、それっぽくカウントした。

成人向けと男性向けは全ての作品に入っていたので除外してる。

const ranks = {};
tags.forEach((tag) => {
  if (ranks[tag] === undefined) ranks[tag] = 0;
  ranks[tag] += 1;
});
ranks['成人向け'] = 0;
ranks['男性向け'] = 0;
const topRanks = Object.entries(ranks).sort((a, b) => b[1] - a[1]).slice(0, 20);

結果

こんなジャンルが人気あるみたいですよ。

  console.log test/dmm.test.js:52
    [
      [ '中出し', 77 ],
      [ '巨乳', 63 ],
      [ 'フェラ', 56 ],
      [ '新作', 47 ],
      [ 'おっぱい', 44 ],
      [ '制服', 36 ],
      [ 'パイズリ', 35 ],
      [ '寝取り・寝取られ・NTR', 27 ],
      [ '準新作', 22 ],
      [ '処女', 20 ],
      [ 'アナル', 20 ],
      [ '人妻・主婦', 19 ],
      [ '野外・露出', 18 ],
      [ 'ラブラブ・あまあま', 16 ],
      [ '辱め', 14 ],
      [ 'ぶっかけ', 14 ],
      [ '近親相姦', 13 ],
      [ '和姦', 11 ],
      [ '3P・4P', 11 ],
      [ 'ハーレム', 11 ]
    ]

 PASS  test/dmm.test.js (687.794s)
  ✓ doujin (686607ms)