RailsのFormクラスについて

Railsでバリデーションを書くケースといえばビジネスロジックで場所はModel(ActiveRecord)が多いのではないかと思います。

これは、たとえばRails Tutorialで扱うような、フォームで扱うModelが一つかつ単純な場合には特に迷う必要はありません。RailsのフォームでsubmitされたパラメータをModelに渡してそのままActiveRecordのバリデーションを使うことができます。

しかし、実際のアプリケーションでは、たとえば住所+配送方法など複数のModelをユーザーの利便性を考えたりすると一つのフォームで扱うことはありうるでしょう。

こういう場合、submitされたパラメータを処理するController側がModelに対応するようにパラメータを編集するか、Model側がフォームの中身を知ってパラメータの形式とModelの対応のコードを引き受けるか、ということになりがちです。

個人的には(Webブラウザという)クライアントからくるリクエストパラメータの形を見てビジネスロジックが扱うクラスが処理できる形にして渡すのはどちらかというとコントローラ側の仕事であって、ModelがいちいちViewのことを知るな、と思うんですが、どちらにせよ見通しが悪くなりがちです。

そこで、Railsでは具体的なテーブルと対応しないForm用のモデルクラスを用意して、Modelの属性とフォームで入力するパラメータの構成が綺麗にマッチしない部分を処理させる実装のパターンがあり、それに利用するクラスをform objectというようです。

form objectで嬉しいのは

• ActiveRecordと同じバリデーションの作法が使える
• formオブジェクトにActiveRecordのモデルと同様に渡すことができ、バリデーションエラーのメッセージを渡す処理が書きやすい
• これまでActiveRecordのModelに書いたりControllerに書いたりで置き場所が落ち着かなかったロジックをまとめやすい

あたりだと思います。

具体的なコードは参考元を見るとして、コード上の要点としては

• ActiveRecordと同じバリデーションを使うために ActiveModel::Model をinclude
• ActiveRecordと違って対応するスキーマがないので扱うパラメータを明示するために attr_accessor を書く
• form object は form_for に渡せる(バリデーションエラーを渡したりなど便利)

参考

• https://robots.thoughtbot.com/activemodel-form-objects
• http://api.rubyonrails.org/classes/ActiveModel.html
• http://tech.medpeer.co.jp/entry/2017/05/09/070758

短いですが、今週の現場からは以上です。

RailsのModelのvalidatesメソッドを見て単純なバリデーションテストを生成するgemを書きました

github.com

表題の通り、RailsのModelのvalidatesメソッドを見て単純なバリデーションテストを生成するgemを書きました。

書いた理由

RailsのModelの基本的なバリデーションのテストは時々、Rails Tutorialやドキュメントのコピペと何が違うのだろう、これはActiveRecordの方のテストになるから書いてないよという話もある一方、書かないと心配で結局手動でテストしてしまうよね...と虚しくなってしまう時があります。

結局心配で手動でテストをするならば、黙って書けばよい気がしますが、自動テスト化の流れが遅れているというモバイルであっても、こういったサービスが出てこようとする中、自分はこの手のテストに時間を使ったり満足していたりしていていいのか、少しもやもやしました。

そうして、どうせいつか誰かに自動化されるのだったら、たまにはフレームワークから離れて、コピペ元のテンプレートを用意したらあとは自動的にてきとうなテストコードを出してくれるライブラリを書いて、自分で半自動化してみようか、ということで書きました。

まぁ、実際に使うにはテンプレートをもっと精査する必要があるのですが...。

使い方

class Sample < ActiveRecord::Base
  validates :name, length: { maximum: 32, minimum: 6 }, presence: true
  validates :id, numericality: { only_integer: true, greater_than: 0 }

  # ...
end

irb(main):001:0> Tamebou::Writer.new("../path/to/model/sample.rb").write
=============================================
class TestSample < MiniTest::Unit::TestCase
  def setup
    @sample = build(:sample)
  end

  def teardown
    # you can write some clean up code in this method
  end

  def test_valid_name_values_in_terms_of_length
    expected_values = ["aaaaaaa", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"]
    expected_values.each do |expected_value|
      @sample.name = expected_value
      assert @sample.valid?
    end
  end

  def test_invalid_name_values_in_terms_of_length
    invalid_values = ["aaaaaa", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"]
    invalid_values.each do |invalid_value|
      @sample.name = invalid_value
      assert_not @sample.valid?
    end
  end
end
=============================================
=============================================
class TestSample < MiniTest::Unit::TestCase
  def setup
    @sample = build(:sample)
  end

  def teardown
    # you can some clean up code in this method
  end

  def test_valid_name_values_in_terms_of_presence
    expected_values = ["presence"]
    expected_values.each do |expected_value|
      @sample.name = expected_value
      assert @sample.valid?
    end
  end

  def test_invalid_name_values_in_terms_of_presence
    invalid_values = [nil]
    invalid_values.each do |invalid_value|
      @sample.name = invalid_value
      assert_not @sample.valid?
    end
  end
end
=============================================
=============================================
class TestSample < MiniTest::Unit::TestCase
  def setup
    @sample = build(:sample)
  end

  def teardown
    # you can some clean up code in this method
  end

  def test_valid_id_values_in_terms_of_numericality
    expected_values = [2, 1]
    expected_values.each do |expected_value|
      @sample.id = expected_value
      assert @sample.valid?
    end
  end

  def test_invalid_id_values_in_terms_of_numericality
    invalid_values = ["1.1", 2.0, 0]
    invalid_values.each do |invalid_value|
      @sample.id = invalid_value
      assert_not @sample.valid?
    end
  end
end
=============================================
=> #<File:../path/to/model/sample.rb (closed)>

デフォルトはMinitestのコードが出てくる設定なので、RSpec(らしい)コードを出したい場合:

Tamebou::Writer.new("../path/to/model/sample.rb", Tamebou::Writer::DefaultTemplate::RSPEC).write

setupメソッドを書き足しておきたい場合やMinitestだけどminitest-spec-railsなどを使っているので、自分で用意したテンプレートを追加痛い場合は、

Tamebou::Writer.new("../path/to/model/sample.rb", "/path/to/your_template").write

うまくパースされてないかもしれないからログを出したい場合:

Tamebou::Writer.new("../path/to/model/sample.rb", Tamebou::Writer::DefaultTemplate::RSPEC, true).write

ちなみに、専用のテンプレートが対応していない場合は、

# 現在対応していません
validates :username, :format => /\A([^@\s]+)@((?:[a-z0-9]+\.)+[a-z]{2,})\Z/i

=============================================
class TestSample < MiniTest::Unit::TestCase
  def setup
    @sample = build(:sample)
  end

  def teardown
    # you can some clean up code in this method
  end

  def test_valid_username_values_in_terms_of_format
    expected_values = ["YOU SHOULD ADD EXPECTED VALUES BY YOUR OWN"]
    expected_values.each do |expected_value|
      @sample.username = expected_value
      assert @sample.valid?
    end
  end

  def test_invalid_username_values_in_terms_of_format
    invalid_values = ["YOU SHOULD ADD UNEXPECTED VALUES BY YOUR OWN"]
    invalid_values.each do |invalid_value|
      @sample.username = invalid_value
      assert_not @sample.valid?
    end
  end
end
=============================================

のような出力が出ます。

中身

正規表現である程度書いて諦めたりしています。

色々プロダクションで動くコードじゃないから、とりあえず１日で作ってみようと手を抜いた箇所が多くてあれです。

参考にさせていただいた記事など

morizyun.github.io qiita.com qiita.com qiita.com qiita.com Rubyist Magazine - 標準添付ライブラリ紹介 【第 10 回】 ERB

OmniAuth(omniauth-twitter)での認証が401 Authorization Required invalid_credentialsで失敗する

OmniAuth系のライブラリでTwitterやFacebookなどのサービスと連携しようとしたところ、401 Authorization Required invalid_credentialsでひたすら失敗してつらかったです。

特にTwitterのAPIには、401でないエラーを401で返すという話もあり*1、結局関連するソフトウェアや設定を一つずつ検討することになりました。。

これを忘れて繰り返すと非常に辛いので、この場で一つずつ簡潔にまとめておきます。

https://github.com/omniauth/omniauth/wiki/FAQ#how-to-fix-oauthunauthorized-error-for-twitter-provider

検討事項

• OAuthっぽいこと
  • アカウント情報
  • Twitterアプリの作り直し
  • ブラウザの時刻
• memcached関連
  • memcachedサーバとの接続
  • provider_ignores_stateでsessionの値を確認を回避
  • memcachedのstats
• omniauthの設定
  • OmniAuth.config.full_host

OAuthっぽいこと

アカウント情報

Twitterアプリなどで、OAuth用のアプリケーションの権限、利用ドメインなどの設定があればその登録、リダイレクトURI、アプリケーションのIDとシークレットなどの確認をします。

Twitterアプリの作り直し

参考: http://qiita.com/hirokishirai/items/5a43977a38ecd922bfb9

どうしてもうまくいかないときはTwitterアプリケーションを作り直すとうまくいくことがあるそうです。

ここではTwitterアプリの作り直しを試したりしました。

連携先のサービスによってはOAuth用アプリケーションごとに一人のユーザーを異なるアカウントIDで管理していたりするので、新しく連携させるわけでないときは注意しましょう*2。

ブラウザの時刻

Twitterでは、0.1秒でもブラウザの時刻がずれていると認証に失敗してしまいます(アプリをエミュなどでテストをする際は割と辛い...)。

こちらのページなどでご利用のパソコンでの時刻にずれがないか確認できます http://www.nict.go.jp/JST/JST5.html

memcached関連

401のエラーを吐いている箇所のコードはこちらのようになっています。

!options.provider_ignores_state && (request.params["state"].to_s.empty? || request.params["state"] != session.delete("omniauth.state"))

の部分を参考にGoogleしたら、sessionの値がTwitterなどからPOSTされるまで、保持されてないのかもしれない、ということになりました。

自分の環境の場合は、sessionの保存にmemcachedを利用していたので、memcachedの確認をしていました。

memcachedサーバとの接続

とりあえず、pingした。

provider_ignores_stateでsessionの値を確認を回避

• https://github.com/intridea/omniauth-oauth2/issues/32
• https://github.com/tdiary/tdiary-core/issues/494

を参考に request.params["state"] != session.delete("omniauth.state") の判定をそもそも呼ばないようにしてみました。

memcachedのstats

sessionに利用してるmemcachedの利用出来るバイト数などは問題ないか、何かデータがうまく取れてないかなということで、memcachedでstatsを確認してみました。

あんまりよくわかっていないので、get_misses, bytes, limit_maxbytes, accepting_conns, threadsなどを見てみたのですが、それらしい違いはなかったです。

omniauthの設定

config/initializers/omniauth.rbなどのinitializersのファイル中の設定を確認しました。

具体的には、

Rails.application.config.middleware.use OmniAuth::Builder do
  provider :twitter, Rails.secrets.oauth.twitter["app_id"], Rails.secrets.oauth.twitter["app_secret"]
  provider :facebook, Rails.secrets.oauth.facebook["app_id"], Rails.secrets.oauth.facebook["app_secret"]
en

など(自分で書いておいて、おそらくdotenv、利用して環境変数で渡した方がセキュアでしょう)で書いてあると思うのですが、これが動作環境で所望の値が得られているか、railsコンソールで叩いて確認したりしていました。

結局は、OmniAuth.config.full_hostに設定するホストのスキーマがhttpsにしなければいけないところがhttp になっているという話でした。

長々書きましたが、いままでの全部関係なかったのでつらいですね。

SSLに対応している部分と対応してない部分、対応している部分ではhttpsにリダイレクトするような設定なのですっかり頭から抜けていたのですが、リダイレクト先に設定したスキーマに合わせましょう。

デバッグ途中で開発環境で動かしてみたら401にならなかったので、localhostとリモートで何の設定が違うんだ、と眺めていたら発見しました。

*1:http://qiita.com/hirokishirai/items/5a43977a38ecd922bfb9

*2:Facebookなど http://qiita.com/awakia/items/ae760fd0d9248a2a5890

バージョン指定をしない時、Railsのrake db:migrateでchangeメソッドを実行するMigrationのインスタンスの配列をどうやって取得しているのか

migrationで厄介な対応が必要な場合、SchemaMigrationをいじって整合性を取る必要性がある、と先輩が言っていたのを聞いて、ふむふむと追いかけてみました。

なお、この記事に出てくるコードのRailsのバージョンは、5.0.0.alphaです。

まとめ

バージョン指定をしない時、Railsのrake db:migrateでchangeメソッドを実行するMigrationのインスタンスの配列の取得手順について.

• migrationファイルが入ったパスでmigrationファイルを探し、それに対応するMigrationクラスのサブクラスのインスタンスの配列を作る
• 上記をprefixによるバージョンで並び替える
  • upの場合、ActiveRecord::SchemaMigrationに対応したテーブルに入っているレコード数分は飛ばした上記の配列のサブセットを取得
  • downの場合、上記の配列の0番目から指定したバージョンまでのサブセットを取得
• さらに、上のように得た配列の中身に対して、
  • upの場合、もう一度SchemaMigrationのテーブルに入っている値から、その中にすでにmigrationされているものがないか調べて除外
  • downの場合、指定したバージョンを除外した後、SchemaMigrationのテーブルに入っている値だけを選別している

Migrationをする範囲を指定しているらしきメソッドに行き着くまで

rake db:migrateコマンドで実行されている処理を探したいのですが、rake ...で動くということは、libsとかtasksとかに置いてありそうだな、ということで探すと、activte_record/tasks/database_tasks.rbにそれらしいのがありました。

module ActiveRecord
  module Tasks # :nodoc:
    ...
    module DatabaseTasks
      extend self
      ...

      def migrate
        verbose = ENV["VERBOSE"] ? ENV["VERBOSE"] == "true" : true
        version = ENV["VERSION"] ? ENV["VERSION"].to_i : nil
        scope   = ENV['SCOPE']
        verbose_was, Migration.verbose = Migration.verbose, verbose
        Migrator.migrate(Migrator.migrations_paths, version) do |migration|
          scope.blank? || scope == migration.scope
        end
      ensure
        Migration.verbose = verbose_was
      end

これを読んでいけばいいのでしょうか。

と思ったら、意外に変数を受け渡す感じのメソッドが続きます。

def migrate(migrations_paths, target_version = nil, &block)
  case
  when target_version.nil?
    up(migrations_paths, target_version, &block)
  when current_version == 0 && target_version == 0
    []
  when current_version > target_version
    down(migrations_paths, target_version, &block)
  else
    up(migrations_paths, target_version, &block)
  end
end

def up(migrations_paths, target_version = nil)
  migrations = migrations(migrations_paths)
  migrations.select! { |m| yield m } if block_given?

  new(:up, migrations, target_version).migrate
end

def initialize(direction, migrations, target_version = nil)
  raise StandardError.new("This database does not yet support migrations") unless Base.connection.supports_migrations?

  @direction         = direction
  @target_version    = target_version
  @migrated_versions = nil
  @migrations        = migrations

  validate(@migrations)

  Base.connection.initialize_schema_migrations_table
end

Migrationをする範囲を指定しているらしきメソッド

バージョンを指定しなかった場合の挙動に関して知りたい場合、Migration#migrateメソッドの中身以降を読んでいけばいいのかな、という感じでした。

def migrate
  # def target
  #   migrations.detect { |m| m.version == @target_version }
  # end
  # なので、@target_versionがある(=バージョン指定をした)場合は、
  # 指定のバージョンがなかったら、例外を発生して終わる
  if !target && @target_version && @target_version > 0
    raise UnknownMigrationVersionError.new(@target_version)
  end


  runnable.each do |migration|
    Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger

    begin
      execute_migration_in_transaction(migration, @direction)
    rescue => e
      canceled_msg = use_transaction?(migration) ? "this and " : ""
      raise StandardError, "An error has occurred, #{canceled_msg}all later migrations canceled:\n\n#{e}", e.backtrace
    end
  end
end

Migrationをする範囲を指定しているらしきメソッドの中身1

まず、注目したいのは、Migration#runnableメソッドです。

def runnable  
  runnable = migrations[start..finish]
  if up?
    runnable.reject { |m| ran?(m) }
  else
    # skip the last migration if we're headed down, but not ALL the way down
    runnable.pop if target
    runnable.find_all { |m| ran?(m) }
  end
end

このメソッドの返り値で実行するmigrationが決まっています。

runnable = migrations[start..finish]

の行をもうちょっとじっくり眺めてみます。

startの部分

downの場合は0で、upの場合はActiveRecord::SchemaMigrationのテーブルにはいっているレコードの数か0になっています。

def start
  up? ? 0 : (migrations.index(current) || 0)
end

# 関連コード

def current_migration
  migrations.detect { |m| m.version == current_version }
end
alias :current :current_migration

def current_version
  migrated.max || 0
end

def migrated
  @migrated_versions ||= Set.new(self.class.get_all_versions)
end

get_all_versionsの部分ですが、ここがActiveRecord::SchemaMigrationモデルに対応したテーブルを見に行っています。

def get_all_versions(connection = Base.connection)
  if connection.table_exists?(schema_migrations_table_name)
    SchemaMigration.all.map { |x| x.version.to_i }.sort
  else
    []
  end
end

finishの部分

バージョン指定がなければ、migrationsの配列の最後のindexを返します

def finish
  migrations.index(target) || migrations.size - 1
end

def target
  migrations.detect { |m| m.version == @target_version }
end

migrationsの部分

migrationファイルの入ったパスを探しに行って、ファイルから読み込んで作ったActiveRecord::Migrationのサブクラスの配列を作っているっぽいです。

def migrations(paths)
  paths = Array(paths)

  files = Dir[*paths.map { |p| "#{p}/**/[0-9]*_*.rb" }]

  migrations = files.map do |file|
    version, name, scope = file.scan(/([0-9]+)_([_a-z0-9]*)\.?([_a-z0-9]*)?\.rb\z/).first

    raise IllegalMigrationNameError.new(file) unless version
    version = version.to_i
    name = name.camelize

    MigrationProxy.new(name, version, file, scope)
  end

  migrations.sort_by(&:version)
end 

以上をまとめると、runnable = migrations[start..finish]は何も指定がない場合、

• migrationファイルが入ったパスでmigrationファイルを探し、それに対応するMigrationクラスのサブクラスのインスタンスの配列を作る
• 上記をprefixによるバージョンで並び替える
  • upの場合、ActiveRecord::SchemaMigrationに対応したテーブルに入っているレコード数分は飛ばした上記の配列のサブセットを取得
  • downの場合、上記の配列の0番目から指定したバージョンまでのサブセットを取得

しているようです。

Migrationをする範囲を指定しているらしきメソッドの中身2

そうして取得したMigrationのサブクラスのインスタンスについてどうしているかというと、

  if up?
    runnable.reject { |m| ran?(m) }
  else
    # skip the last migration if we're headed down, but not ALL the way down
    runnable.pop if target
    runnable.find_all { |m| ran?(m) }
  end

    def ran?(migration)
      migrated.include?(migration.version.to_i)
    end

• upの場合、もう一度SchemaMigrationのテーブルに入っている値から、その中にすでにmigrationされているものがないか調べて除外
• downの場合、指定したバージョンを除外した後、SchemaMigrationのテーブルに入っている値だけを選別している

こういうコードになってたんですね。

ActiveRecord::Baseのcreateとsaveの違い

たまたま仕事で気になったので、ActiveRecord::Baseのcreateとsaveの違いについて簡単に調べました。

まとめ

• save(!)メソッドはcreate(!)メソッドで利用されている
• create(!)メソッドはnewメソッドとsave(!)メソッドを行うメソッド
• save(!)メソッドを利用しているので、レコードの保存に失敗した場合、createメソッドでは属性が代入されたオブジェクトが返り、create!メソッドでは例外が発生する
• save(!)メソッドの中を追っていくと、new_record?メソッドの返り値によって、INSERT文を呼び出しているメソッドとUPDATE文を呼び出しているメソッドを分けている風に見える

save(!)メソッドはcreate(!)メソッドで利用されている

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/persistence.rb より。

def create(attributes = nil, &block)
  if attributes.is_a?(Array)
    attributes.collect { |attr| create(attr, &block) }
  else
    object = new(attributes, &block)
    object.save
    object
  end
end

(中略)

def create!(attributes = nil, &block)
  if attributes.is_a?(Array)
    attributes.collect { |attr| create!(attr, &block) }
  else
    object = new(attributes, &block)
    object.save!
    object
  end
end

create(!)メソッドはnewメソッドとsave(!)メソッドを行うメソッド

def create!(attributes = nil, &block)
  if attributes.is_a?(Array)
    attributes.collect { |attr| create!(attr, &block) }
  else
    object = new(attributes, &block)
    object.save!
    object
  end
end

save(!)メソッドを利用しているので、レコードの保存に失敗した場合、createメソッドでは属性が代入されたオブジェクトが返り、create!メソッドでは例外が発生する

save!メソッドとsaveメソッドのコードを見てみる

# 例外が発生しても捉えられているので、
# createのメソッドのコードは続行するため、createメソッドでは
# 保存に成功していない状態のオブジェクトが返ってくる
def save(*args)
  create_or_update(*args)
rescue ActiveRecord::RecordInvalid
  false
end

# 例外が発生する
def save!(*args)
  create_or_update(*args) || raise(RecordNotSaved.new("Failed to save the record", self))
end

save(!)メソッドの中を追っていくと、new_record?メソッドの返り値によって、INSERT文を呼び出しているメソッドとUPDATE文を呼び出しているメソッドを分けている風に見える

save(!) メソッドの中にあるcreate_or_update(*args)を探してみる。

def create_or_update(*args)
  raise ReadOnlyRecord, "#{self.class} is marked as readonly" if readonly?
  result = new_record? ? _create_record : _update_record(*args)
  result != false
end

なるほど...。

論理削除カジュアルに触発されて現場で見たデータベースについて考えた事を書いてみた

論理削除カジュアルに触発されたので、社会人になってここ半年くらい、現場で見たデータベースについて考えた事について書いてみます。

内容

• とりあえず外部キーにはインデックスを貼っておく
• ユニーク制約はデータベースレベルでやらないとしばしば崩れる
• 対称なテーブルには対称にindexを貼る
• serializableやacts_as_bitの列は一行でも検索のコードを書いたら別テーブルに括り出す

とりあえず外部キーにはインデックスを貼っておく

どういう傾向かはよく分からないのですが、アプリケーションのスキーマを見ていると、 deleted_atとの複合インデックスや、アプリケーション内のコードから呼び出される組み合わせのindexは貼ってあるのですが、 外部キーに対してインデックスを貼っていないことがあります*1。

いわゆる外部キーぎらいという奴です。

アプリケーションがよく動いているなら、まあ、いいじゃないか*2、とも思います。

しかし、調査のためにいつもと違うSQLを書くと、JOINのときの結合先の行を選ぶ際にインデックスが効かないせいでtemp落ちしたり、時間がかかりすぎるのでabortされたりします。

外部キーは、けっこうカーディナリティが小さいいいキーになることが多い気がしているので、迷ったらつけておいていいと思います*3

正直な感想として、セカンダリインデックスに凝らずとも外部キーだけでも問題ない場合も多い気もしています。

ユニーク制約はデータベースレベルでやらないとしばしば崩れる

ユニーク制約嫌いというのもありますが、ユニーク制約のバリデーションはある程度負荷がかかるサービスなら、アプリケーション層のバリデーションだけで行うと、それなりに崩れる*4ということを学んだので、データベース側にもユニーク制約をつけた方がよいと思います。

ユニークにしたいキーによって外部サービスと連携する予定があるなら、外部サービスはこちらのキーの重複なんて考慮してくれませんので、絶対つけたほうがよいです。

対称なテーブルには対称にindexを貼る

たとえば少し前に流行ったSTIのようなことをやる場合*5*6、 STIで扱うような同じカテゴリに属するものを扱う複数のテーブルがあるとき、これらのテーブルに貼ってあるindexが非対称なとき、結構めんどくさいことになります。

たとえば、こんな感じです。

create_table :text_books do
  t.integer  "user_id",
  t.string   "title"
  t.integer  "author_id"
  t.integer  "publisher_id"
  t.text     "description"
  t.datetime "created_at"
  t.datetime "updated_at"
end

add_index "text_books", ["user_id", "title"], name: "index_text_books_on_user_id_title", unique: true, using: :btree
add_index "text_books", ["user_id", "author_id"], name: "index_text_books_on_user_id_title", unique: true, using: :btree
add_index "text_books", ["user_id", "publisher_id"], name: "index_text_books_on_user_id_title", unique: true, using: :btree

create_table :comics do
  t.integer  "user_id",
  t.string   "title"
  t.integer  "author_id"
  t.integer  "publisher_id"
  t.text     "description"
  t.datetime "created_at"
  t.datetime "updated_at"
end

add_index "comics", ["title", "user_id"], name: "index_comics_on_user_id_title", unique: true, using: :btree
add_index "comics", ["author_id", "user_id"], name: "index_comics_on_user_id_title", unique: true, using: :btree
add_index "comics", ["publisher_id", "user_id"], name: "index_comics_on_user_id_title", unique: true, using: :btree

text_booksとcomicsで共通して同じクエリを使おうとすると、片方のindexが効かなくなって、 ほとんど同じなのにそれぞれ用にクエリを用意する必要が出てきて少し困ります。

複数の列で絞り込みかけたいとかいう話になってくるとめんどくさくなって、共通してキーに入ってきそうなキーが第一キーでないindexが貼ってある方は完全に諦めたりします*7。

serializableやacts_as_bitの列は一行でも検索のコードを書いたら別テーブルに括り出す

ある程度カテゴリーでまとまった情報を扱う列を作って、その列の中身をserializable細かい情報は全部底に入れてしまおう、という列がしばしばあります。

いささかひどい例ですが、以下のようなもの。

create_table :users do
  t.string   "email",
  t.string   "account_id"
  t.text     "detail"
  t.string   "authorities"
  t.datetime "created_at"
  t.datetime "updated_at"
end

class User < ActiveRecord::Base
  acts_as_bit :authorities, %(eat drink listen rest)
  serialize :detail, JSON

  def address
    detail[:address]
  end

  def secret_question
    detail[:secret_question]
  end
end

何が怖いかをいくつかあげてもいいのですが、本題ではないので*8。

これは、

• かなり検索性が悪い
• 複数のユーザー間でフォーマットが統一される事が期待できない

ということが想定されます。

検索性については、エラスティックサーチを入れれば、というのがあるのですが、何文字目が1/0のユーザーを捜すためにエラスティックサーチを入れるのは大仰な気がします。

また、detailの方、つまりもっと大きいハッシュを扱っている列ならいいのか、というと、そうでもなく、落ち着いてコードを眺めると、ある程度共通したフォーマットで扱いたいデータの組がありそうです。

そして、そういった組はテーブルに括り出した方が扱いやすいです。

最初はレコード間で大きなシリアライズされた列の中にある共通する列はよく見えない可能性もありますし、authoritiesのような列は検索もしないかもしれません。

なので、見える見ないではなくて、アプリケーションコードの中で、その列をWHERE句に入れて*9、SELECT文をかけるようなコードを書いたら、テーブルやカラムに括り出せないか検討するといいと思います。

コードが多くないうちはいいのではないか、という話ですが、コードが増えると想像以上に修正をする気が失せ、 また、それと同時にその形式のレコード数が増えて修正が難しくなるので、そういうコードを見かけたタイミングで考えてみるのでよいと思います。

そういえば、アンケートの列を週一くらいのペースでほいぽい付け替えたい、みたいな要求を受けた事があります。

当時は無理矢理LIKEで書いていたました*10。

しかし、いまはむちゃくちゃな件数溜まる前にアンケートの形式が決まる事を信じて、 絶対固定の部分と固定じゃない部分をはっきりさせた後、 固定じゃない部分はtextかblobの列に突っ込んでおいてElasticSearch入れて運用とかそういうことを考えたほうがまだましかもなと思います。

もう少しネタはあった気はしますが、遅くなったので今日はここまでにします。

*1:私も一回やらかしました。ごめんなさい...

*2:タイムアウトになるときがあるから頭が痛いのですが

*3:というより、Railsの人たちの中でつけない感じがあるだけで、外部キー制約をつけたらついてくるDBMSもあるというかMySQLはそうです

*4:アプリケーションサーバが重くて、ユーザに連打され複数のサーバで同じリクエストを受けつけ、しかもそれぞれのサーバでモデル層のユニーク制約のバリデーションを通過してから、DBに重複したSQLのリクエストを送信されるという感じ

*5:これはテーブルが共通なので表題にあげた問題は起こらないのですが

*6:別にSTIがいつもいいわけではなく、NULL列が多発するならマッパーの部分書き換えてでもCTIに変形していったほうがいいと思います。最初からCTIは仕様が決まりきっている場合でなければ帯に短したすきに長しをやりがちなのでよした方が

*7:初心者なので許してください

*8:たとえば、誤ってauthoritiesに要素を追加するときに順序を間違えると途端に動かなくなります。戻せば動くようになるという話ではなく、戻すまで、入れ替わった順序に基づいて文字列が更新されてしまうので、取り返しがつかなくなると思います

*9:特に、正規表現を使って

*10:ひぇー

Railsのコントローラのアクション間でデータの状態の管理がよく分からないとき

こちらの記事*1の続きです。

なんか、いわゆるfat controllerの具体的事例集、みたいなかんじになってしまって途中で書く気が失せてきたのですが、ふと思い出して自分の公開鍵のメモをいま見直したら予想以上に良かったので、一応残しときます。

内容

• アクション間でデータの状態の管理がよく分からないとき
  • アクションの間でのデータの受け渡しがよく分からない
    • 本当にややこしいのは情報の状態管理ではないか
    • 情報を送り合う時は送り合う方法をなるべく統一する
  • 登録フォームの中で外部サービスにアカウントを作るためのパラメータを入力する欄がある
    • 自サービスの登録ボタンを押した際の外部サービスへのリクエストの結果による登録失敗が嫌
      • 外部サービスのアカウントを自サービスのレコードの前に作成してしまう
    • レスポンスの結果だけを扱うクラスを用意して責務を分担する
    • そもそも失敗率はどれくらいなのか

アクション間でデータの状態の管理がよく分からないとき

複数のアクションで、１つの注文というレコード(とそれにまつわるレコード)を作ろうとした時のことを考えてみます。

商品選択 => 住所入力 => 決済情報入力 => 注文確認 => 注文確定

みたいな感じの奴です。

これらのアクションの間で入力されたデータを受け渡すのは基本的に割と難しい気がします。*2

なんで難しいのか、と外部APIへのリクエストがこの流れに含まれているとさらに難しくなる場合があるので、その場合について対策をメモしておきます。

アクションの間でのデータの受け渡しがよく分からない

まず、外部APIとかそういうことは置いておいて、自サービス内のことだけを考えてみます。

商品選択 => 住所入力 => 決済情報入力 => 注文確認 => 注文確定

の流れで情報を送る場合、あるいは、この処理について修正をする場合に億劫になる理由について、以下の本を読んでたら、何となく触れられていました。

まだ解決法まで読めていないのでそういうことがあるんだ、としか言えない感じではありますが、結構長い本なのでたどり着く前に忘れそうです。なので、いまのところ考えていたことをまとめておきます。

Patterns of Enterprise Application Architecture (Addison-Wesley Signature Series (Fowler))

本当にややこしいのは情報の状態管理ではないか

最初に謝っておきますと、本の中で触れられていたのは、複数のコントローラのアクションを使った遷移ではないです。

本の中で触れられていたのは、アプリケーションサイドで１つのレコードを作るまでの入力 ~ 登録までの状態を管理することが難しい(そしていくつかの解決方法がある)、ということでした。

フォームの画面遷移は、レコードの管理のような側面から言い直せば、先の方の画面で登録に必要な情報を入力されてから、最後の画面で送信ボタンを押して登録するまでのトランザクションの管理、とも言えます。

やっていることは、いわゆるアプリケーションサイドでのトランザクションのような感じなんですね*3。

ここをいじるのは本当にややこしいこった、みたいな文章を読んで自分だけではないのだ、と若干安心したのでした*4。

情報を送り合う時は送り合う方法をなるべく統一する

とはいえ、処理がややこしいので、大きなフォームをどーんと出して情報をいっぺんに入力してくれ、とは言えません。何かしら工夫する必要があります。

なんとなくではありますが、自分がいじっててややこしかったのは、前入力した情報をどこで持っているか、ということでした。

複数のアクション間で情報を受け渡すにはいくつかの方法があります。

cookie, session, hidden要素を含んだフォームの送信, 一部だけデータベースに前もって保存してデータベースから読み込んだり、といった場合もあるでしょう。

いくつかある情報を複数の画面間で受け渡すとき、上記のものを組み合わせて用いられていて、方法が統一されていないのがしんどかったです。

なので、sessionなりフォームなり、画面遷移の際、情報を受け渡す方法を一定にしておくと少しはやりやすいのかな、と思いました。

また、最近だとJavaScriptのMVCフレームワークのModel層でもバリデーションがやりやすいため、それを利用したり、フォームの切り替えをサーバサイドでなく、クライアントサイドでやるというアプローチ*5もあります。

登録フォームの中で外部サービスにアカウントを作るためのパラメータを入力する欄がある

商品選択 => 住所入力 => 決済情報入力 => 注文確認 => 注文確定

この流れの中で、実は外部サービスと提携していて、どこかで外部APIへの登録を行わないといけないということも考えられます。

自サービスの登録ボタンを押した際の外部サービスへのリクエストの結果による登録失敗が嫌

ここで厄介なのが、自サービスに入力していた情報は正常だったはずなのだけれど、それを元に外部サービスのAPIを叩いたら、通信エラーかなにかで登録に失敗した、ということです。

これを避けたいので、上の遷移だと 決済情報入力 の分で、注文は確定していないのですが注文のためのアカウントを作成してしまう、それを決済情報入力のバリデーションの一貫としてしまう、ということが考えられます。

正直な話をすると、これは悪手だと思います。

後から、ジョブなりを回して注文に至らなかったけれど作成されている一部のデータを削除する事はもちろん可能です*6。

けれど、本当は注文が確定するタイミングとアカウントが作成されるタイミングは可能な限り揃えられるならその方が扱いが楽になります*7。

問題は、外部サービスAPIのリクエストにこけるのが嫌、こけた場合どうしたらいいか分からないなので、まずこの2点を考えたらいいのではないでしょうか。

レスポンスの結果だけを扱うクラスを用意して責務を分担する

成功以外のレスポンスが怖くて、レスポンスが成功を受け取った場合以外処理を先に進めたくない場合、とりあえず、外部APIのレスポンスが大きい、というより本当は

「フォームを投稿してレコードを作成する一環として外部APIのアカウントを取得する」

という処理の単位が一カ所で扱うには大きすぎるのではないか、ということが考えられます。ですので、まず外部APIと通信を行う部分を外部サービスアカウント専用のモデルやクライアントを用意して対処すると良いと思います。

そのモデルやクライアントの中で、レスポンスが失敗した場合のレスポンス等を想定した、リクエストの送信と結果の受け取りをテストします。そして、コントローラのアクション側ではモデルのレスポンスが成功した場合と失敗した場合だけを考えるようにすると少しは楽になるかもしれません。

そもそも失敗率はどれくらいなのか

成功しなかった場合が怖いと言っておりますが、実際どれくらいの失敗率なのでしょう。

一回すでにそのAPIを使っている人が周りにいるなら相談してみるといいと思います。

測ったり聞いたりした結果、数百回やって1回も発生しないようであれば、UIへの影響は杞憂ではないでしょうか。

一旦実装はその例外を無視して9割方を完成させてしまって、気持ちに余裕ができてからきちんと取り組んだ方が経験的によさそうです*8。

*1:アクションのコードがやたらと縦に長いとき

*2:最初に書く時はそうでもないのですが、追加や編集がね...

*3:ユーザーから見た一まとまりのデータ管理の単位をbusiness transactionと言っていたような気がします

*4:そして、レビュアーに気づいたら大きくなりすぎていたPRについてごめんなさいを言いに行ったのでした

*5:この場合、投稿される情報について、それぞれの項目ごとにオブジェクトを用意しているのかね...

*6:ただ、ジョブで本当に削除されているのか、コントローラのアクションを読んだ人は割と長い間不安になるのではないでしょうか。まあ、正直、私が苦手でしてね。

*7:うまく説明できないのだけど、トランザクションは0か無か、にしておいたほうがよいと思います

*8:一度に考える事は１つの方がよい結果を生む気がします。